[Tutor] code review

[Cameron Simpson]

On 12Jun2014 00:38, Alan Gauld <alan.gauld at btinternet.com> wrote:
>On 11/06/14 11:43, Adam Gold wrote:
>># create snapshot names like the following: 2014-06-10T01-00-01.vm1.img.bz2
>>for i in vgxenList:
>>     DATE = datetime.datetime.now().strftime("%Y-%m-%d" + "T" + "%H-%M-%S")
>
>Why use addition? You could just insett the literal T...
>
>DATE = datetime.datetime.now().strftime("%Y-%m-%dT%H-%M-%S")
>
>Also using all caps for the name suggests a constant by convention.
>
>>     vgxenName = "/dev/vgxen/"
>>     lvName = i

Just further to these two snippets, "/dev/vgxen/" is just the kind of thing you 
would treat as a "constant" (Python doesn't really have constants, just the 
convention that names in all UPPERCASE represent stable ad hoc values which 
won't change, and would be "constants" in some other languages.)

So at the top of your script (after imports) you might have:

   VXGEN_DIR = "/dev/vgxen"

and use it later.

Note, no trailing slash in that dir name. In view of this, later where you 
append to backupList you might go:

   snap_path = os.path.join(VXGEN_DIR, snapName)
   backupList.append(snap_name)

os.path.join is the usual way to assemble pathnames form path components; it is 
in principle platform independent.

BTW, someone has proably mentioned it, but in case not: names like this_that 
are the convention for normal variables in Python as opposed to thisThat 
camelcase names. The main use of camelcase in Python names is class names "This 
That".

It doesn't affect your code in any functional way, but others will find you 
code slightly more readable.

These common conventions are outlined in PEP 8:

   http://legacy.python.org/dev/peps/pep-0008/

Worth a read. It is only strict for stdlib module source code, but adherence is 
common in most code.

Cheers,
Cameron Simpson <cs at zip.com.au>

A good catchword can obscure analysis for fifty years.