[Tutor] playing around with function

[Danny Yoo]

>    def navigated(self):
>        if (self.state == 'NF') and
> (self.store.units[int(self.id)].isfuzzy()):
>            return True
>        if (self.state == 'PF') and
> (self.store.units[int(self.id)].isfuzzy()):
>            return True
>        if (self.state == 'NT') and
> (self.store.units[int(self.id)].istranslated()):
>            return True
>        if (self.state == 'PT') and
> (self.store.units[int(self.id)].istranslated()):
>            return True
[code cut]


This code here looks highly repetitive, and there's really nothing bigger 
than a case analysis going on here.  I'd strongly recommend using a data 
structure to record the core essense of this code.

One possible refactoring is:

########################################
navigationTable = {'NF': 'isfuzzy',
                    'PF': 'isfuzzy',
                    'NT': 'istranslated',
                    ...}
test = navigationTable(self.state)
unit = self.store.units[int(self.id)])
if test == 'isfuzzy':
     return unit.isfuzzy()
elif test == 'istranslated':
     return unit.istranslated()
...
########################################

The heart of that code is the mapping between states and the function to 
call at that state, so we can keep that knowledge centralized in a data 
structure.


I get the feeling that the remainder of the code is really too closely 
tied with GUI stuff; there's the finite-state automata thing that I see 
here, but it's entangled with Qt widget logic.  You may want to consider 
how to break the GUI stuff out of the navigational code.

>    def navigationNext(self,state):
>        if (self.getCurrentItem() == 0):
>            return 0
>        self.id = int(self.item.text(0))
>        self.id += 1
>        for i in range(self.id,775):
>            self.id = i
>            if (i == 775):
>                pass
>                #self.warningMessage()


Other comments: the condition testing for i == 775 seems impossible to me. 
775 is outside of the for loop's domain.  That code won't ever fire.

navigationNext() and navigationPrevious() look like the exact same 
function.  The only significant difference I see is the increment or 
decrement used to move 'id' around.  Have you considered turning that into 
a parameter?