1
\$\begingroup\$

I'm taking my first ever CS class and I have an assignment due Friday. I just wanted someone to check my code.

Instructions:

Write a program that:

  1. gets the name of a text file of numbers from the user. Each number in the file is on its own line.

  2. reads in those numbers one at a time

  3. writes the even numbers to a file named even.txt

  4. writes the odd numbers to a file named odd.txt

  5. displays to the user the sum of the positive numbers and the count of the negative numbers.

Here is what I have:

def main():
    #Open text file for reading
    fileName   = input('Enter name of a file of numbers (include the whole path): ')
    numberFile = open(fileName, 'r')

#Priming read
    number = numberFile.readline()

#Setting up loop to continue reading until
#an empty line is reached
    total = 0
    count = 0
    while number != '':
        number = float(number)                  #convert from string to number
        if number%2 == 0:                       
            evenNumber = open('even.txt', 'w')  #writes even numbers into a file
            evenNumber.write(number + '\n')
        else:
            oddNumber  = open('odd.txt', 'w')   #writes odd numbers into a file
            oddNumber.write(number + '\n')

        for number in numberFile:
            number = float(number)      #convert from string to number

            if number <= 0:             #identify negative numbers
                count += 1              #count negative numbers

            if number >= 0:             #identify positive numbers
                total += number         #sum of positive numbers
        number = numberFile.readline()

    numberFile.close()                  #close file after program is complete

    print('The count of the negative numbers is: ', count)
    print('The sum of the positive numbers is: ', total)

main()

Example of a "file of numbers" (each number in a new line):

25
18
-7
4.2
99
103
-100
\$\endgroup\$

closed as off-topic by 200_success Aug 4 '16 at 7:48

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ Can you give an example of what the "file of numbers" is supposed to look like? \$\endgroup\$ – syb0rg Aug 4 '16 at 4:31
  • 1
    \$\begingroup\$ Have you tested this program? I'm not convinced that this code, as posted, works. (And one requirement of Code Review questions is that you need to be reasonably certain that the code works when posting the question.) For one thing, the displayOutput() function is defined inside the main() function, but it is never called. \$\endgroup\$ – 200_success Aug 4 '16 at 4:38
  • \$\begingroup\$ Sorry! I fixed it as much as I know how. I also included an image of what the input file would look like. \$\endgroup\$ – j.davis Aug 4 '16 at 4:44
2
\$\begingroup\$

First, welcome to codereview. I can see a couple of things that can be improved in your code and I'll start with some style guides (PEP8) which are good to follow when writing python code.

Comments:

Block comments should start with #space

You've missed the space all over, after #

  1. More, regarding comments, they shouldnt tell the reader what the code does but why it does it

  2. Another aspect is that you have way too many comments when they're not really necessary, but if this is a requirement, I think it might work as well.

  3. It's necessary to have only two spaces and then write a comment, not tons of useless empty spaces. So:

    number = float(number) # convert from string to number

Should become:

number = float(number)  # convert from string to number

Spacing:

  1. After / Before each operator you should have an empty space:

number%2 == 0 -> number % 2 == 0

  1. Don't add to many spaces before =. You might think this makes the code look better but it just makes it harder to read:

oddNumber = open('odd.txt', 'w') -> oddNumber = open('odd.txt', 'w')

Naming conventions

Variable names should be lowercase, with words separated by underscores as necessary to improve readability.

evenNumber -> even_number

You can also use mixedCase like you did if you're being constant.

Now, having said this, we have:

def main():
    # Open text file for reading
    file_name = input('Enter name of a file of numbers (include the whole path): ')
    number_file = open(file_name, 'r')

    # Priming read
    number = number_file.readline()

    # Setting up loop to continue reading until an empty line is reached
    total = 0
    count = 0
    while number != '':
        number = float(number)  # convert from string to number
        if number % 2 == 0:
            even_number = open('even.txt', 'w')  # writes even numbers into a file
            even_number.write(number + '\n')
        else:
            odd_number = open('odd.txt', 'w')  # writes odd numbers into a file
            odd_number.write(number + '\n')

        for number in number_file:
            number = float(number)  # convert from string to number

            if number <= 0:  # identify negative numbers
                count += 1  # count negative numbers

            if number >= 0:  # identify positive numbers
                total += number  # sum of positive numbers
        number = number_file.readline()

    number_file.close()  # close file after program is complete

    print('The count of the negative numbers is: ', count)
    print('The sum of the positive numbers is: ', total)


main()

Moving forward, analyzing the code:

The code doesn't work !

Your code is not working, and it shouldn't:

TypeError: unsupported operand type(s) for +: 'float' and 'str'

Here:

even_number.write(number + '\n')

You're trying to add a new-line character (which is str) to a float. That doesn't work. When adding things next to each other in Python, first of all you have to make sure that they are of the same type

Here:

number_file = open(file_name, 'r')

You should use with open() here. The advantage of using a with statement is that it is guaranteed to close the file no matter how the nested block exits. If an exception occurs before the end of the block, it will close the file before the exception is caught by an outer exception handler.

So, you will have:

with open(file_name) as number_file:  # it opens the file in read mode by default
    ...

The same rule applies for the other two files.

DON'T READ WHAT'S WRITTEN BELOW BUT RATHER TRY TO LISTEN TO THE OTHER ANSWERS AS THEY GIVE NICE ADVICES


Let's take the requirements point by point and build our program:

  1. gets the name of a text file of numbers from the user. Each number in the file is on its own line.
def get_user_input():
    file_name = input('Enter name of a file of numbers (include the whole path): ')
    if os.path.isfile(file_name):
        return file_name
    else:
        print('The path is not correct!')
        sys.exit(0)

Above, we made sure the user enters a correct path and after this check, if everything went well, return the name of the file. Else, print a message and exit.

The next point:

  1. reads in those numbers one at a time
  2. writes the even numbers to a file named even.txt
  3. writes the odd numbers to a file named odd.txt
  4. displays to the user the sum of the positive numbers and the count of the negative numbers.

Now, we need to read one line at a time, then add the numbers to different files if they are odd or even.

BUT be carefull:

"Only integers are even or odd. We do not call fractions, irrational numbers, imaginary numbers, and so on either even or odd"

So, for this to work, if we find a non-integer, we will throw a message to the user to let him know he has an invalid number in his file. However, I don't know how you need to treat this, but I'll just do like I said above. Be aware that this can be handled by multiplying the float number with 10...0 depending on where the decimals start from.

import os
import sys

def get_user_input():
    file_name = input('Enter name of a file of numbers (include the whole path): ')
    if os.path.isfile(file_name):
        return file_name
    else:
        print('The path is not correct!')
        sys.exit(0)


if __name__ == '__main__':
    file_name = get_user_input()
    negative_numbers = []  # this list will hold negative numbers from file
    positive_numbers = []  # this list will hold positive numbers from file
    with open(file_name) as numbers_file, open('odd_numbers_file', 'w') as odd_numbers_file, open('even_numbers_file', 'w') as even_numbers_file:
        for line in numbers_file:
            if float(line) < 0:
                negative_numbers.append(float(line))
            else:
                positive_numbers.append(float(line))

            if float(line) % 2 == 0:
                even_numbers_file.write(line)
            elif float(line) % 2 == 1:
                odd_numbers_file.write(line)
            else:
                print('This line: {0} does not contain an integer'.format(line))
                continue
    print('The count of the negative numbers is: {0}'.format(len(negative_numbers)))
    print('The sum of the positive numbers is: {0}'.format(sum(positive_numbers)))

Things that I added:

if __name__ == '__main__':. The reason I did this is that you can also import your code as a module in another script and then run the main function if and when your program decides. I created two lists, and appended what was needed to each of them. The way you did it, it's not optimal as you're looping through your file multiple times.

There are special cases for this to handle, which I don't since there's no explicit requirement for them:

  • 0 as an odd / even number
  • float numbers
  • float numbers which might be treated as integers: e.g. 54.0000, 23.00
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your help! I was able to fix the problem by doing this: even_number.write(str(number) + '\n') and changing the <= and >= to just < and >. The reason for all the spaces: my professor wants us to line up the equal signs. He says it makes it easier for users to read. We aren't suppose to use exception handling in this assignment (we only just learned that and are suppose to do the project for it later). \$\endgroup\$ – j.davis Aug 4 '16 at 19:13
  • \$\begingroup\$ Although what you've should me definitely makes it easier to understand and do, unfortunately I can't use it before we haven't learned any of it. This is a basic 110 level class. \$\endgroup\$ – j.davis Aug 4 '16 at 19:48
2
\$\begingroup\$

Some quick comments:

  • At least as written, your code doesn’t work for me. I created the following file:

    1
    

    and passing it to your script got the following error:

    Enter name of a file of numbers (include the whole path): numbers2.txt
    Traceback (most recent call last):
      File "parsenumbers.py", line 37, in <module>
        main()
      File "parsenumbers.py", line 20, in main
        oddNumber.write(number + '\n')
    TypeError: unsupported operand type(s) for +: 'float' and 'str'
    

    I’m using Python 3.4.3 on OS X.

  • I fixed that (fix hidden for you to work it out yourself):

    Cast number to str(number) before writing.

    Then I created a longer file:

    1
    2
    3
    4
    5
    

    And ran the script again. Still broken:

    # odd.txt
    1.0
    
    # even.txt
    (Empty)
    

    It’s also strange that 1 has become 1.0 in the output file. I’d expect it to be carried through unchanged – what’s happened?

Once you’ve got the code working, some style comments: * Run your code through a PEP 8 linter. This will catch style issues like variable names, whitespace, etc.

  • Everything is wrapped up in one massive main() function. That makes this code difficult to test and reuse – it would be better if you had separate functions, each of which took a list of numbers, and would do different things.

    e.g. one function gets the sum of the negatives, another the positives, another to write the evens to a file.

  • Rather than calling main() at the top-level, put it in an if __name__ == '__main__' block. This means it only runs if the script is called directly – and other people can import from your script without side effects.

  • Use with open(filename) as f:, not f = open(filename) … f.close(). This is more idiomatic, and safer – you can’t leave files open accidentally. (You never actually close even.txt or odd.txt after you’ve opened them!)

  • None of your comments tell me anything I couldn’t get from reading the code. They should explain why the code is written this way – how does it relate to the problem it’s trying to solve – not what it’s doing. And as a sidebar, it’s really annoying if comments don’t line up with the code they’re describing.

  • Use better variable names. You have count and total, but it’s not obvious what these correspond to. How about negative_count and positive_total – otherwise it sounds like you’re getting a count/total for the entire set.

  • Your code assumes that zero is negative, and will count it as such. That’s somewhat counter-intuitive: instinct tells me that zero is neither positive nor negative, but there are times when it’s more complicated.

    This would be a really good spot for a comment: if it’s not completely ambiguous, state what choice you’re making (in plain English), and why you’ve chosen to do it this way.

\$\endgroup\$
2
\$\begingroup\$

This code is not working with quite a lot of bugs in it, and as such I'd say it's not ready for a review. My guess is that you wrote the code but didn't actually run it.

Since this is an assignment I don't want to do your homework for you, but I'll give you some hints:

  • Check the difference between input and raw_input
  • What if the file to open doesn't exist or has wrong permissions?
  • What if you can't open the output files (maybe because of permissions)?
  • What does opening a file with the w flag exactly do?
  • When you write to a text file what kind of data do you expect to write?
  • How many files do you open and how many do you close?
  • Is zero both positive and negative? That means in the total it would be counted twice?
  • Check variable names, maybe count and total are a bit misleading as to how you use them
\$\endgroup\$
  • \$\begingroup\$ -1 for raw_input op specifically said python3, where input is raw_input \$\endgroup\$ – Oscar Smith Aug 4 '16 at 12:54
  • \$\begingroup\$ We aren't allowed to use raw_input. We never learned that. \$\endgroup\$ – j.davis Aug 4 '16 at 19:49

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