[Tutor] Code critique

[Peter Otten]

Bo Morris wrote:

> "...Regarding your program, instead of writing long sequences of
> repetitive if
> conditions, I would write one function for each of the different
> operations and store them in a dict, mapping each host name to a function
> (and multiple host names may map to the same function). Then, look up the
> host name in the dict and call the corresponding function to run the right
> operations on that host..."
> 
> Thank you Stefan for your input. Would please be willing to provide an
> example?

Start small. Look at this

>     if '3102EHD-Lanka-1108' in out:
>         s.exec_command('export DISPLAY=:0.0; cd /Downloads/Hourly/win.sh')
>         sftp = s.open_sftp()
>         sftp.get('/Downloads/Hourly/3102EHD-01108/3102EHD-01108.png',
> '/Downloads/Hourly/3102EHD-01108.png')
>         sftp.close()
>         print 'file recieved'
>     elif '3102EHD-01109' in out:
>         s.exec_command('export DISPLAY=:0.0; cd /Downloads/Hourly/win.sh')
>         sftp = s.open_sftp()
>         sftp.get('/Downloads/Hourly/3102DHD-01109/3102DHD-01109.png',
> '/Downloads/Hourly/3102DHD-01109.png')
>         sftp.close()
>         print 'file recieved'
> 

and try to move the code in the if-suite into a function

>     if '3102EHD-Lanka-1108' in out:
          process()

Then try to determine the argument(s) needed to make the same process() 
function work for both the if and the elif suite:

>     if '3102EHD-Lanka-1108' in out:
          process(?)
>     elif '3102EHD-01109' in out:
          process(?)

Once that works see if you can preprocess `out` so that it only contains the 
hostname. Do you still need the if...elif...elif...? 

Come back for more hints if you think you do.

Random remarks:

> command = 'echo $HOSTNAME'

The sooner you remove unused code, the better.

> cd /Downloads/Hourly/win.sh

That looks like you are cd-ing into a shell script.

> con = map(connect, l)

That should be a for loop; also, use a descriptive variable name instead of 
`l`.

PS: a dict would come into play once you want to treat machines differently. 
Say, your computers are called one, two, three, four, ..., and you have a 
default procedure

def process_default(hostname): ...

and some special treatment for three, seven, and eight:

def process_three(hostname): ...
def process_seven_or_eight(hostname): ...

Then you can build a lookup table

lookup_process = {
    "three": process_three,
    "seven": process_seven_or_eight,
    "eight": process_seven_or_eight,
}

and find the applicable function with

hostname = ...
process = lookup_process.get(hostname, process_default)
process(hostname)