> 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.
_______________________________________________
Tutor maillist - Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor
