Cleaning up code

[jem00]

hey again,

I wrote this code.. very quickly and messily.
I would like to 'clean' it up.. that is put some functions in it.

def numWords(aString):
    lst = aString.split()
    noword = 0
    for word in lst:
        noword = noword + 1
    return noword

def save


inp = raw_input("Please enter some words: ")
total = 0

total = numWords(inp)
words = "File has %d words" % total
print words
    
choice = raw_input("Would you like to save the file?(y/n): ")

if choice == 'y' and 'ye' and 'yes':
    save = str( raw_input("What would you like to save it as.(file.txt: "))
    import os
    
    if os.path.exists(save):
        overwrite = raw_input("Would you like to overwrite file?")
        
        if overwrite == 'y' and 'ye' and 'yes':
            f = file(save,'w')
            f.write(inp + '\n' + words)
            f.close()
            



        else:
            save2 = raw_input("Would you like to save it under a different name? ")
            
            if save2 == 'y' and 'ye' and 'yes':
                 save = str( raw_input("What would you like to save it as.(file.txt: "))
                 f = file(save,'w')
                 f.write(inp + '\n' + words)
                 f.close()
                 

                
    else:
        f = file(save,'w')
        f.write(inp + '\n' + words)
        f.close()
        
else:
    print "Discarding unsaved changes..."
    raw_input("Press Enter to exit.......")

Can someone give me some ideas, but not give it away so that I may try and figure it out myself?

THANKS,

Jem

[sneekula]

The first function can be cleaned up to:

def numWords(aString):
    return len(aString.split())

# test it
print numWords("Merry Christmas to everyone")  # 4

You can do more cleanup here:

choice = raw_input("Would you like to save the file?(y/n): ")

if choice in ['y', 'ye', 'yes']:
    print 'yes'  # test

# or even better
if 'y' in choice.lower():
    print 'yes'  # test

Your line 'def save' is of course an incomplete function statement.

[jem00]

hey thanks for the reply,

I was wondering if my code was too mesy with the branched if statements

How may I amend this?

Jem

[katharnakh]

you can also put repeating lines doing same work in a fuction. In your case saving to a file, in a function.

def saveFile(fSave):
	f = file(fSave,'w')
	f.write(inp + '\n' + words)
	f.close()

kath.

[jrcagle]

Just a miscellaneous thing: all import statements belong at the top of the file (rule of thumb, can be broken if needed, but usually good).

I was wondering if my code was too mesy with the branched if statements

Yes, a bit. But it's not terrible, and sometimes unavoidable.

How may I amend this?

First, consider what the nested conditionals do. Essentially, they are trying to get the user to give a unique filename.

That action can be turned into a separate function, like so:

def get_valid_filename(prompt):
   while True:
       filename = raw_input(prompt)
       if filename == "cancel":
            return ""
       if os.path.exists(filename):
          query = raw_input("That file exists.  Overwrite? (y/n) ")
          if query == 'y':
             return filename
...
choice = raw_input("Would you like to save the file?(y/n): ")

if choice in ['y','ye','yes']:   # NOTE THE FIX HERE!
    filename = get_valid_filename("Enter a filename: ")
    if filename:
       saveFile(filename)
else:
    print "discarding changes...."

etc.

There are two advantages to throwing your "get file" code in a separate file.

First, you can debug the get_valid_filename separately and be confident in its correct operation without having to worry about how it interacts with the rest of your code. This is called the "interface model" -- basically, all that the rest of your code has to worry about is passing the right arguments to the function.

Second, the main code is now short, sweet, and easy to read.

And third, get_valid_filename can be stashed in a codebase somewhere and reused in the future.

Jeff