Simple python code takes command line argument for file location and tokenizes text

Question

The usage of this Python program is:

python ~/test.py test.txt 

Here is the source:

import sys #Open file passed by terminal if len(sys.argv)==2: try: open(sys.argv[1]) with open(sys.argv[1]) as inputfile: #Iterate through lines for line in inputfile: #tokenize words=line.split() #print tokens separated by commas print(",".join(words)) inputfile.close sys.exit(0) except: sys.exit(-1) sys.exit(-1) 

This is the "test.txt"

\ / input(" ") input(print("some text")) one two three other things here more stuff on this line garbage \n %20 

and it should output (which it does):

\,/,input(",") input(print("some,text")) one,two,three other,things,here more,stuff,on,this,line garbage \n %20 

But I'm wondering if there is some unexpected way to break it. Is there a way to nest carriage returns or something somewhere? Oh, and I realize that I can accomplish the same thing with:

print(",".join(line.split())) 

But for this specific instance, I would like to separate the two steps.

Answer 1

You have a few redundant and unnecessary calls in there.

1. Calling open(sys.argv[1]) right before with open(sys.argv[1]) as inputfile: does not do anything.
2. The with statement already takes care of closing the file. So you don't need the inputfile.close
3. If with did not close your file, it would not be closed, because you actually need to call that method (so inputfile.close())
4. You should never have a bare except. This catches more than you want, including, for example, the user pressing Ctrl+C. Always use as specific an exception as possible. Here you probably want to except the file missing, so use either IOError (Python 2.x) or FileNotFoundError (Python 3.x)

You should also have a look at Python's official style-guide, PEP8. It recommends surrounding operators with spaces, so if len(sys.argv) == 2:.

If you want this script to do something more, you should start to split the responsibilities into functions. Here you could have something like this:

import sys def parse_lines(file_name): """Parse the lines of file_names into comma-separated lists""" with open(file_name) as inputfile: for line in inputfile: yield ",".join(line.split()) if __name__ == "__main__": # Open file passed by terminal if len(sys.argv) == 2: try: for line in parse_lines(sys.argv[1]): print(line) except FileNotFoundError: print(f"File {sys.argv[1]} not found") sys.exit(-1) 

Here I put the parsing into a function, which is actually a generator. It yields the parsed lines one by one. I also added a docstring describing the function.

I also made your error handling code more specific. Your code actually makes debugging your program harder, because the program returning -1 (regardless of the error encountered) is a lot less clear than Python creating a stacktrace. If you do catch errors, you should handle them (and give the user feedback if necessary). This uses Python 3.6's new f-strings.