[Tutor] please give me feedback - linux & virtual machine python script

[eryksun]

On Sun, Nov 4, 2012 at 1:43 AM,  <pygods at hush.com> wrote:
>
> There are more features I like to implement but It would be nice to get
> constructive feedback from the community.
> Here is the code http://pastebin.com/R8b8M5PR

On line 104 you print an error message to stdout instead of stderr and
then call sys.exit(1). You can print the message to stderr and set the
return code to 1 at the same time:

    sys.exit("Pynotify Python module does not seem to be installed")

On line 131 you're checking if an object equals None. This isn't
idiomatic. None is a singleton, so when you use None as a sentry or
default option, it's faster and more reliable to test identity (is, is
not) instead of equality or boolean value. Here's a toy example where
testing equality fails:

    >>> class Equal(object):
    ...     def __eq__(self, other):
    ...         return True
    ...
    >>> Equal() == None
    True
    >>> Equal() is None
    False

On line 148 you're not taking advantage of os.getenv's default return
value of None. Also, you're manually splitting on '\n' instead of
using the splitlines() method:

        uris = os.getenv("NAUTILUS_SCRIPT_SELECTED_URIS")
        if uris is not None:
            return uris.splitlines()

On line 166 you use a list comprehension to replace some text in uris
and then immediately iterate over the result in a for loop. Just move
the replace operation into the loop:

        uris = self.nautilus()
        for uri in uris:
            uri = uri.replace(self._home_path, SHARE_NAME)

Lines 249 and 259 should be "elif" statements. Also, I doubt the
following does what you think it does:

            while not vmc.vbox_is_running() and range(45):

Each evaluation of the expression creates a new range(45) list. Did
you want a counter here?