6
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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. \$\endgroup\$ – Billal Begueradj Dec 12 '17 at 9:02
  • \$\begingroup\$ 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. \$\endgroup\$ – Jeremy H. Dec 12 '17 at 9:07
  • \$\begingroup\$ 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. \$\endgroup\$ – Toby Speight Dec 12 '17 at 18:10
  • \$\begingroup\$ @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. \$\endgroup\$ – Jeremy H. Dec 12 '17 at 18:56
5
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.