Newbie: Looking for code review on my first Python project.

[Chris Angelico]

On Wed, Jan 11, 2012 at 10:44 AM, HoneyMonster
<someone at someplace.invalid> wrote:
> Hi,
>
> I'm new to Python and recently completed my first project. I used
> wxPython with wxGlade to generate the GUI bits.The application seems to
> work well, but I am entirely self-taught, so have undoubtedly committed a
> number of howlers in terms of style, design, standards, best practice and
> so forth.

Welcome!

Ian has already offered some excellent tips, so I'll not repeat him.


    log = os.environ['HOME'] + "/log/bbc.log"
    log = os.environ['HOMEPATH'] + "\\log\\bbc.log"

Python on Windows will support / for paths; I'd then break out the
HOME / HOMEPATH check into a separate variable (eg 'basepath'), and
then only that and icondir would need to be in the 'if Linux' block.


about = "Built by Walter Hurry using Python and wxPython,\n" + \
        "with wxGlade to generate the code for the GUI elements.\n" + \
        "Phil Lewis' get_iplayer does the real work.\n\n" + \
        "Version 1.05: January 10, 2012"

I'd do this with a triple-quoted string:

about = """Built by Walter Hurry using Python and wxPython,
with wxGlade to generate the code for the GUI elements.
Phil Lewis' get_iplayer does the real work.

Version 1.05: January 10, 2012"""


        # Configure the logging
        # Generate a list for the PVR queue

Comments like this aren't much use, although the second would be a
good place to expand "PVR".

                # We only want the PID at the start if the line, and
it is always 8 bytes

Much more useful :)


        self.add = wx.MenuItem(self.file, wx.NewId(), "&Add to Queue",
"Add a programme to the queue (for download later)", wx.ITEM_NORMAL)
        self.file.AppendItem(self.add)

I don't have/use wxpython so I can't say for sure, but I think
AppendItem returns the item appended. This allows you to avoid
repeating yourself:

        self.add = self.file.AppendItem(wx.MenuItem(self.file,
wx.NewId(), "&Add to Queue", "Add a programme to the queue (for
download later)", wx.ITEM_NORMAL))

This prevents mismatching assignment and append, when you copy/paste/edit etc.

Decent bit of code. I've seen far worse... and not from new programmers :)

Chris Angelico