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