Thanks a lot for the review Sascha, I will improve these things.<br><br><div class="gmail_quote">On Tue, Aug 21, 2012 at 9:38 AM, Sascha Silbe <span dir="ltr"><<a href="mailto:silbe@activitycentral.com" target="_blank">silbe@activitycentral.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Martin Abente Lahaye <<a href="mailto:martin.abente.lahaye@gmail.com">martin.abente.lahaye@gmail.com</a>> writes:<br>
<br>
[src/jarabe/journal/volumestoolbar.py]<br>
[_set_up_activities_examples()]<br>
[...]<br>
<div class="im">> + activities = []<br>
> + for home_activity in home_model._activities:<br>
> + if home_activity.is_journal():<br>
> + continue<br>
> +<br>
> + activity_name = home_activity.get_activity_name()<br>
> + if activity_name not in activities:<br>
> + self._add_activity_example(home_activity)<br>
> + activities.append(activity_name)<br>
<br>
</div>I wonder if using a dictionary for deduplication would be better<br>
here. E.g.:<br>
<br>
activities = dict([(home_activity.get_activity_name(), home_activity)<br>
for home_activity in home_model])<br>
for home_activity in activities.values():<br>
self._add_activity_examples(home_activity)<br>
<br>
<br>
Alternatively, you could just iterate over all entries and let<br>
_add_activity_examples() take care of deduplication (as it has to do<br>
anyway), at the risk of making it O(n²).<br>
<br>
<br>
> + def _proper_occurrence(self, home_activity):<br>
<br>
What's a "proper" occurrence?<br>
<div class="im"><br>
> + if home_activity.is_journal():<br>
> + return False<br>
> +<br>
> + home_model = shell.get_model()<br>
> + act_name = home_activity.get_activity_name()<br>
> + activities = filter(lambda act: act_name == act.get_activity_name(),<br>
> + home_model._activities)<br>
> +<br>
> + return len(activities) <= 1<br>
<br>
</div>I assume this is for deduplication again. The easiest and fastest way<br>
overall (O(1) per iteration [1], O(n) in total) is probably a<br>
ref-counting scheme:<br>
<br>
def __init__(self, ...):<br>
# ...the usual stuff...<br>
<br>
self._activity_counts = {}<br>
self._example_buttons = {}<br>
for home_activity in home_model:<br>
self._add_activity(home_activity)<br>
<br>
def _add_activity(self, home_activity):<br>
"""Keep track of an active activity session<br>
<br>
Keep track of an active activity session and add the examples<br>
folder if available and not already shown.<br>
"""<br>
name = home_activity.get_activity_name()<br>
old_count = self._activity_counts.get(name, 0)<br>
self._activity_counts[name] = old_count + 1<br>
if old_count:<br>
return<br>
<br>
# ...create button...<br>
<br>
self._example_buttons[name] = example_button<br>
<br>
def _remove_activity(self, home_activity):<br>
"""Stop keeping track of an active activity session<br>
<br>
Remove all internal references to this session. Remove the<br>
corresponding examples folder if it's the last instance of<br>
this activity.<br>
"""<br>
name = home_activity.get_activity_name()<br>
old_count = self._activity_counts[name]<br>
self._activity_counts[name] = old_count - 1<br>
if old_count > 1:<br>
return<br>
<br>
example_button = self._example_buttons.pop(name)<br>
<br>
# ...remove button...<br>
<div class="im"><br>
<br>
<br>
> + def _add_activity_example(self, home_activity):<br>
> + examples_path = os.path.join(home_activity.get_bundle_path(),<br>
> + 'examples')<br>
> +<br>
> + if not os.path.exists(examples_path):<br>
> + return<br>
<br>
</div>If you add a guard against home_activity.get_bundle_path() returning<br>
None, you can drop the special-casing for the Journal activity.<br>
<br>
<br>
Sascha<br>
<br>
[1] <a href="http://wiki.python.org/moin/TimeComplexity#dict" target="_blank">http://wiki.python.org/moin/TimeComplexity#dict</a><br>
<span class="HOEnZb"><font color="#888888">--<br>
<a href="http://sascha.silbe.org/" target="_blank">http://sascha.silbe.org/</a><br>
<a href="http://www.infra-silbe.de/" target="_blank">http://www.infra-silbe.de/</a><br>
</font></span></blockquote></div><br>