[Tutor] Engarde program was: i++

[Danny Yoo]

> There where some values I did not want saved to the character file.  A couple 
> where values that are for all characters, so I put them into their own class.
>
> ###############################
> class Master_stats:
>     def __init__(self, year, month):
>         self.year = year
>         self.month = month
> ###############################
> There are only two values now, but that will most likely increase as I get 
> further into the program.


Hi Scott,


Sounds good.  Ok, let's re-review the code now.


####################################################################
def mfile_read():
     '''This will open the master file and load all the variables'''
     i = 0
     [some code cut]

     master = cPickle.load(mfile)
     mfile.close()
     return [master.year, master.month]
####################################################################

In the context of the new Master_stats class, it makes more sense for 
mfile_read() to return a Master_stats object now rather than a two-tuple 
list.  You already had instincts about bundling year and month together 
here, which is good, so if you follow through on this, it'll let you 
dissolve a few more lines of legacy code.



###################################################
def char_save(character):
     '''This saves the character to the harddrive'''
     path = "./characters/" + character.player
     cfile = open(path, "w")
     cPickle.dump(character, cfile)
     return
###################################################

To make the code a little safer, explicitely close the file here.  The 
Python language itself doesn't say that 'cfile' here will be closed at the 
end of char_save() --- the fact that it does happen is an implementation 
detail, since Jython does something different! --- so it's better just to 
avoid the possible ambiguity and explicitely close files resources.



Ok, looking at create_char() since it's one of the larger functions.  The 
value of 's' in create_char() is mixed: at one point, it's an integer, at 
other instances, a string.  There's a possible flow of control that 
doesn't make sense to me.

##########################################################
     s = 0
     while s < 1 or s > 3:
         print 'Is', player, 'the player you wanted?'
         s = raw_input()
         if s == 'y' or s == 'yes':
             break
         elif s == 'n' or s == 'no':
             return
         else:
             print '\nplease select y, n, yes, or no\n'
     while s < 1 or s > 3:
         ## code cut
##########################################################

The condition at the last line there should be a type error: 's' at the 
end is a string, and strings should not be comparable to numbers. 
Unfortunately, Python suffers a defect here: it is way too lax in this 
area, so something consistent (but meaningless!) is going to happen here.


Also, around this code, I see I lot of code duplication.

##############################################################
     while s < 1 or s > 3:
         print '\nIs', name, 'a sufficient character name?'
         s = raw_input()
         if s == "y" or s == "yes":
             break
         elif s == "n" or s == "no":

             while s < 1 or s > 3:
                 print '\nIs the alternate name', alt_name,
                 print 'a sufficient character name?'
                 s = raw_input()
                 if s == "y" or s == "yes":
                     name = alt_name
                     break
                 elif s == "n" or s == "no":
                     return
                 else:
                     print '\nplease select y, n, yes, or no\n'

             break

         else:
             print '\nplease select y, n, yes, or no\n'
##############################################################

The duplication is the logic of first prompting a question, then repeating 
the question until we get a straight yes-or-no.  The type mistake with 's' 
is probably a side effect of a bad copy-and-paste.  Let that be a lesson 
to you!  *grin*


But the fix isn't just to fix the type error everywhere: the real fix is 
to get rid of the copy-and-pasting altogether.  Make a function to handle 
the asking of yes/no style questions.  Write a function called is_yes() 
which takes in a question string, and returns True or False.  If we put on 
our "wishing makes it so" hat, here's what we'd like:

##################################
>>> is_yes("did you like this?")
did you like this?
huh?
please select y, n, yes, or no
I don't want to
please select y, n, yes, or no
oh all right
please select y, n, yes, or no
y
True
##################################

So is_yes() should be written to keep asking, and at the end, return True 
if the user affirmed the choice, and False otherwise.


If you have an is_yes() helper function, then the logic of asking all 
those questions dissolves down to:

##############################################################
if not is_yes("Is " + name + " a sufficient character name?"):
     if is_yes("Is the alternate name " + alt_name +
               " a sufficient character name?"):
          name = alt_name
     else:
         return
...
##############################################################

This should make the logic of name selection much clearer than it is right 
now.  At the moment, it's cluttered with the other auxiliary logic you're 
using to make sure the user types in "yes" or "no", and it makes it hard 
to see the intent of the code.


If you have questions about any of this, ask, and one of us here on Tutor 
will be happy to talk about this more.