Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!gegeweb.org!de-l.enfer-du-nord.net!feeder1.enfer-du-nord.net!newsfeed.eweka.nl!eweka.nl!feeder3.eweka.nl!newsfeed.xs4all.nl!newsfeed6.news.xs4all.nl!xs4all!post.news.xs4all.nl!not-for-mail
Return-Path: <rosuav@gmail.com>
X-Original-To: python-list@python.org
Delivered-To: python-list@mail.python.org
X-Spam-Status: OK 0.001
X-Spam-Evidence: '*H*': 1.00; '*S*': 0.00; 'subject:Python': 0.05; 'decent': 0.07; 'elements.': 0.07; 'forth.': 0.07; 'subject:code': 0.07; 'wxpython': 0.07; 'python': 0.08; 'block.': 0.09; 'prevents': 0.09; 'sure,': 0.09; 'configure': 0.10; 'am,': 0.12; 'gui': 0.13; 'received:209.85.210.174': 0.13; 'received:mail- iy0-f174.google.com': 0.13; '"with': 0.16; '(eg': 0.16; 'appended.': 0.16; 'from:addr:rosuav': 0.16; 'from:name:chris angelico': 0.16; 'mismatching': 0.16; 'string:': 0.16; 'style,': 0.16; 'subject:Newbie': 0.16; 'wed,': 0.17; 'wrote:': 0.18; 'bytes': 0.18; 'jan': 0.19; 'seems': 0.20; "aren't": 0.21; 'header :In-Reply-To:1': 0.22; 'code': 0.25; 'code.': 0.26; 'windows': 0.26; "i'm": 0.26; 'variable': 0.28; 'separate': 0.28; 'bit': 0.28; 'work.': 0.28; 'message-id:@mail.gmail.com': 0.28; 'second': 0.29; 'queue': 0.30; 'chris': 0.30; "i've": 0.31; "i'll": 0.31; 'version': 0.32; 'does': 0.32; 'hi,': 0.32; 'list': 0.32; 'break': 0.32; 'programmers': 0.32; "can't": 0.32; 'to:addr:python-list': 0.34; 'assignment': 0.34; '(for': 0.35; 'skip:" 10': 0.37; 'but': 0.37; 'received:google.com': 0.37; 'think': 0.37; 'using': 0.38; 'him.': 0.38; 'allows': 0.38; 'some': 0.38; 'comments': 0.38; 'received:209.85': 0.38; 'useful': 0.38; 'skip:o 20': 0.38; "i'd": 0.39; 'received:209': 0.40; 'to:addr:python.org': 0.40; 'skip:s 40': 0.40; 'download': 0.60; 'more': 0.61; 'committed': 0.64; 'home': 0.65; 'programme': 0.66; '2012': 0.67; 'hurry': 0.67; '11,': 0.68; "'if": 0.84; '10:44': 0.84; 'undoubtedly': 0.84; 'yourself:': 0.84
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=J2FrveTQJ5JWl0PHolS2CwCflw3g8vSF72fmNJUO0vc=; b=eC4qsqkBDXYLM1icyzkvRXLp3wVGKH3lcranVGS5PM3WZ4SI8to44iW1EXxKrsXHgN XZVVOGo7dA63r7LNzEFq8y0VAAVWmLZNCfQEv/cvRDc0Q033Yyt9JYZKmTjz2y38Yc0v IcepKvhE1uobo5bzeI23vTFhdDof5orKcdfWs=
MIME-Version: 1.0
In-Reply-To: <jeiig6$csj$1@news.albasani.net>
References: <jeiig6$csj$1@news.albasani.net>
Date: Wed, 11 Jan 2012 12:43:13 +1100
Subject: Re: Newbie: Looking for code review on my first Python project.
From: Chris Angelico <rosuav@gmail.com>
To: python-list@python.org
Content-Type: text/plain; charset=ISO-8859-1
X-BeenThere: python-list@python.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: General discussion list for the Python programming language <python-list.python.org>
List-Unsubscribe: <http://mail.python.org/mailman/options/python-list>, <mailto:python-list-request@python.org?subject=unsubscribe>
List-Archive: <http://mail.python.org/pipermail/python-list>
List-Post: <mailto:python-list@python.org>
List-Help: <mailto:python-list-request@python.org?subject=help>
List-Subscribe: <http://mail.python.org/mailman/listinfo/python-list>, <mailto:python-list-request@python.org?subject=subscribe>
Newsgroups: comp.lang.python
Message-ID: <mailman.4621.1326246196.27778.python-list@python.org>
Lines: 66
NNTP-Posting-Host: 2001:888:2000:d::a6
X-Trace: 1326246196 news.xs4all.nl 6845 [2001:888:2000:d::a6]:53182
X-Complaints-To: abuse@xs4all.nl
Xref: x330-a1.tempe.blueboxinc.net comp.lang.python:18795

On Wed, Jan 11, 2012 at 10:44 AM, HoneyMonster
<someone@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