How to make this simple code look better

[Tim Chase]

On 2015-10-27 17:24, Ganesh Pal wrote:
> from myPopen import run
> 
> def configure_network():
>     """
>     Prepare network for test
>     """
>     try:
>         cmd = ("netadm enable -p ncp DefaultFixed")
>         out, err, ret = run(cmd, timeout=60)
>         if ret != "":
>             logging.error("Can't run %s got %s (%d)!" % (cmd, err,
> ret)) return False
>         cmd = ("ipadm create-ip net3")
>         out, err, ret = run(cmd, timeout=60)
>         if ret != "":
>             logging.error("Can't run %s got %s (%d)!" % (cmd, err,
> ret)) return False
>         cmd = ("ipadm create-addr -a 192.168.84.3/24 net3")
>         out, err, ret = run(cmd, timeout=60)
>         if ret != "":
>             logging.error("Can't run %s got %s (%d)!" % (cmd, err,
> ret)) return False
>         cmd = (" route -p add default 192.168.84.1")
>         out, err, ret = run(cmd, timeout=60)
>         if ret != "":
>             logging.error("Can't run %s got %s (%d)!" % (cmd, err,
> ret)) return False
>     except Exception, e:
>         logging.exception("Failed to run %s got %s" % (cmd, e))
>         return False
>     logging.info("Configuring network .Done !!!")
>     return True
> 
> 
> Q1.How to make this code look better (in terms of quality)

I'd be tempted

1) to put it in a loop
2) just test the "ret" string directly instead of comparing it to
   the empty string
3) to do your exception testing on each command rather than wrapping
   the entire loop

  for cmd in [
      "netadm enable -p ncp DefaultFixed",
      "ipadm create-ip net3",
      "ipadm create-addr -a 192.168.84.3/24 net3",
      "route -p add default 192.168.84.1",    
      ]:
    try:
      out, err, ret = run(cmd, timeout=60)
      if ret:
        logging.exception("Can't run %s got %s (d)!", cmd, err, ret)
        return False
    except Exception, e:
      logging.exception("Failed to run %s", cmd)
      return False
  logging.info("Configuring network done.")
  return True

It reduces the redundant code and also brings all of the commands
together in one place to see the expected steps.

> Q2. Iam using the except clause, just to maintain the syntax,  will
> any exception be caught in this case.

All regular exceptions will be caught.  I think certain errors (rather
than exceptions) may still behave properly.

-tkc