Code review comment for lp:~zematynnad/ubuntu-webcatalog/duplicates_987826

Revision history for this message
Anthony Lenton (elachuni) wrote :

Hi Danny,

I really don't think the code is doing what we want there. Comments about the changes in src/webcatalog/views.py:
 - You iterate over all of top_rated_apps (tens of thousands of apps) to keep just the first settings.NUMBER_TOP_RATED_APPS off the filtered list (tens of apps). See if there's a way to only filter enough apps as we need.
 - For each package name you currently keep the last item on the list that matches the name (as the dict is overridden each time you find a new Application instance with the same package name), so I you're effectively keeping the *lowest* rated app instance for each package name.
 - Finally, you're storing the apps in a dict and then asking for values(), so you can't expect any order in top_rated_apps.
 - And, something really minor, Count is being imported unnecessarily.

Kind regards,

achuni.

review: Needs Fixing

« Back to merge proposal