Code review comment for lp:~mblayman/entertainer/translate-events

Samuel,

> > def is_empty(self):
> >- """
> >- Is screen history empty.
> >- @return boolean value. True if history is empty, otherwise False.
> >- """
> >+ '''Is screen history empty'''
> > if len(self.screens) == 0:
> > return True
> > else:
> > return False
> >
>
> Maybe a @property makes sense here instead of a method call to fit with the
> recent changes we've made.
> The docstring needs a full stop.

is_empty is now a property (and I threw in a new test case to boot). Here is the partial diff:

matt@eden:~/repo/translate-events/entertainerlib/tests$ bzr diff -r -3..-1
=== modified file 'entertainerlib/frontend/gui/screen_history.py'
--- entertainerlib/frontend/gui/screen_history.py 2009-06-02 01:16:03 +0000
+++ entertainerlib/frontend/gui/screen_history.py 2009-06-03 01:54:03 +0000
@@ -26,8 +26,9 @@
             return None
         return self.screens.pop()

+ @property
     def is_empty(self):
- '''Is screen history empty'''
+ '''Return a boolean indicating if the screen history is empty.'''
         if len(self.screens) == 0:
             return True
         else:

=== modified file 'entertainerlib/frontend/gui/user_interface.py'
--- entertainerlib/frontend/gui/user_interface.py 2009-06-02 01:16:03 +0000
+++ entertainerlib/frontend/gui/user_interface.py 2009-06-03 01:54:03 +0000
@@ -351,7 +351,7 @@

     def _handle_navigate_back(self, event):
         '''Handle UserEvent.NAVIGATE_BACK.'''
- if not self.history.is_empty():
+ if not self.history.is_empty:
             self.move_to_previous_screen()

     def _handle_navigate_home(self, event):

=== added file 'entertainerlib/tests/test_screenhistory.py'
--- entertainerlib/tests/test_screenhistory.py 1970-01-01 00:00:00 +0000
+++ entertainerlib/tests/test_screenhistory.py 2009-06-03 01:54:03 +0000
@@ -0,0 +1,26 @@
+# Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
+'''Tests ScreenHistory'''
+
+from entertainerlib.frontend.gui.screen_history import ScreenHistory
+from entertainerlib.frontend.gui.screens.screen import Screen
+from entertainerlib.tests import EntertainerTest
+
+class ScreenHistoryTest(EntertainerTest):
+ '''Test for entertainerlib.frontend.gui.screen_history'''
+
+ def setUp(self):
+ EntertainerTest.setUp(self)
+
+ self.screen_history = ScreenHistory(None)
+
+ def test_create(self):
+ '''Test correct ScreenHistory initialization.'''
+ self.assertTrue(isinstance(self.screen_history, ScreenHistory))
+
+ def test_is_empty(self):
+ '''Test the is_empty property.'''
+ self.assertTrue(self.screen_history.is_empty)
+ screen = Screen()
+ self.screen_history.add_screen(screen)
+ self.assertFalse(self.screen_history.is_empty)
+