Pearls from the Plone Source (part 1)

07Nov07

Ok, I’m fed up. Here’s the first of a series of Pearls from the Plone Source. Call it The Plone WTF if you like.

Here goes, a loop that does nothing. Notice the ‘locking_info’ adapter is fetched outside the for loop:

for action in workflowActions:
    if action['category'] != ‘workflow’:
        continue

locking_info = queryMultiAdapter((context, request), name=’plone_lock_info’)

And then a few lines below, the same loop repeats, this time it is doing something. The ‘actionUrl’ variable is assigned inside the for loop:

if locking_info and locking_info.is_locked_for_current_user():
    return []

for action in workflowActions:
    if action['category'] != ‘workflow’:
        continue
    actionUrl = action['url']

Sad. I don’t think anyone is really reviewing this code….

Advertisement


3 Responses to “Pearls from the Plone Source (part 1)”

  1. Heh, that’s quite bad. :) Obviously some bad merge/copy-and-paste stuff’s happened here.

    No-one’s perfect, though. I’m sure your code sometimes contains weird errors and omissions too.

    Then again, you could just fix it.

  2. Fix it. That’s why we have unit tests and let people have commit access. :)

    Any code base of Plone’s and Zope’s size will have accidents like this.

  3. 3 PloneUser190

    Unit tests aren’t an excuse for crappy code; the Plone source code is an inspiration for newbies as well as seasoned developers and it’s imperative that we keep a high standard.

    –From the Creators of a now infamous IRC bot.


Leave a Reply

Fill in your details below or click an icon to log in:

Gravatar
WordPress.com Logo

Please log in to WordPress.com to post a comment to your blog.

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Connecting to %s


Follow

Get every new post delivered to your Inbox.