Code review comment for lp:~michael.nelson/ubuntu-webcatalog/987838-no-screenshots-for-extras-2

On Wed, May 9, 2012 at 4:42 PM, Natalia Bidart
<email address hidden> wrote:
> One tiny note, instead of:
>
> if 'Screenshot-Url' in package.candidateRecord:
>    screenshot_url = package.candidateRecord['Screenshot-Url']
>
> I usually prefer something like:
>
> screenshot_url = package.candidateRecord.get('Screenshot-Url')
> if screenshot_url is not None and screenshot_url not in app.screenshots:
>    app.applicationmedia_set.create(media_type='screenshot', url=screenshot_url)
>
> The benefits I see in the second option are:
>
> * you access the package.candidateRecord dict only once
> * you do not duplicate the literal 'Screenshot-Url' and thus avoid risk of typos
> * you avoid an if (and then another inner level in the logic)
>
> So, if you think is worth it, feel free to tweak the code in this MP. I'll be approving anyways since this looks great.

Indeed - thanks! pushed with r123.