Eakin, W wrote:
> The code follows, so any comments and/or suggestions as to what I did
> right or wrong, or what could be done better will be appreciated.
> def fileScramble(fileName):
> newFile = file('scrambled.txt', 'w')
> newRow = ''
> for line in fileName:
> newRow = ''
> tempList = line.split(' ')
> for word in tempList:
> newRow = newRow + ' ' + wordScramble(word)
> newRow = newRow + '\n'
> newFile.write(newRow)
This seem pretty verbose to me. Using tempList doesn't IMO add anything,
it might as well be
for word in line.split(' '):
or
for word in line.split():
since you probably want to split on tabs also and this will strip the
trailing newlines as a bonus.
I usually prefer a list comprehension to an accumulation loop when
possible so I would actually write it as
newWords = [ wordScramble(word) for word in line.split() ]
newRow = ' '.join(newWords) + '\n'
or even
newRow = ' '.join(wordScramble(word) for word in line.split()) + '\n'
though that might be a little too terse for easy comprehension.
> newFile.close()
>
>
> def wordScramble(word):
> punctuation = ['.', ',', ':', ';', '(', ')']
> if len(word) < 4:
> return word
> elif len(word) == 4 and word[-1] in punctuation or word[0] in
> punctuation:
> return word
> elif len(word) == 4:
> word = word[0] + word[2] + word[1] + word[3]
> return word
Rather than repeating the test for len(word) == 4, I would write
elif len(word) == 4:
if word[-1] in punctuation or word[0] in punctuation:
return word
else:
word = word[0] + word[2] + word[1] + word[3]
return word
This also breaks up the long conditional that Danny complained about.
> else:
> (fCut, fLetter) = getLetter(word, 0, 'forward')
> (lCut, lLetter) = getLetter(word, -1, 'back')
> tempWord = list(word)
> del tempWord[:fCut + 1]
> del tempWord[lCut:]
I think this is the same as
tempWord = list(word[fCut+1:lCut])
> random.shuffle(tempWord)
> middleString = "".join(tempWord)
> scrambledWord = fLetter + middleString + lLetter
> return scrambledWord
>
>
> def getLetter(string, number, direction):
I found it very hard to understand what this function does. A better
name and a comment would help a lot. You might consider having separate
getFirstLetter() and getLastLetter() since much of getLetter() is taken
up with the conditionals and compensating for trying to do two things.
def getFirstLetter(string, number=0):
if string[number].isalpha() == True:
return (number, string[:number + 1])
else:
return getFirstLetter(string, number + 1)
Even better would be to use a simple loop to find the index of the first
letter, and split the string into three sections in the caller.
BTW thinking of writing this as a loop brings to mind some problems -
what will your program do with 'words' such as 555-1212 or "Ha!" ?
> if direction == 'forward':
> increment = 1
> else:
> increment = -1
> if string[number].isalpha() == True:
> if direction == 'forward':
> return (number, string[:number + 1])
> else:
> return (number, string[number:])
> elif string[number].isalpha() == False:
> return getLetter(string, number + increment, direction)
>
>
> if __name__ == "__main__":
> try:
> if sys.argv[1].isspace() == True:
> print "No file was given to the program to
> process.\n<----------> Program quitting <---------->\n"
> else:
> try:
> f = open(sys.argv[1])
> fileScramble(f)
> f.close()
> except IOError:
> print "That file does not exist, or you do not have
> permission to access it.\n<----------> Program quitting <---------->\n"
> except IndexError:
> print "No file was given to the program to
> process.\n<----------> Program quitting <---------->\n"
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Tutor maillist - Tutor@python.org
> http://mail.python.org/mailman/listinfo/tutor
_______________________________________________
Tutor maillist - Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor
