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

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.

• 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. Dec 12 '17 at 9:02
• 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. Dec 12 '17 at 9:07
• 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. Dec 12 '17 at 18:10
• @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. Dec 12 '17 at 18:56

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:

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.