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.

• Billal BEGUERADJ: 1. The question is too broad 2. It sounds you are trying to break down an application, and this later one is not funny to ask for it here.
• Jeremy H.: 1. Is there a way to phrase it to be more specific? I am just trying to see if the way that I am validating input is sufficient. 2. Not trying to break an app. I'm trying to break code that I have personally written, because thats one method that I use to improve my coding.
• Toby Speight: I think this could become a good question - it seems that what you want is a review of your test cases, in which case you should post the test program and ask for a review of it.
• Jeremy H.: @TobySpeight I reworded the question and added a couple lines of code for thoroughness. Is this what you were talking about? This is the extent of what I have code and text-wise. I am trying to keep it simple to learn more about this particular part of I/O handling.

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.