8
\$\begingroup\$

I'm making a program that lets you enter a name and house that adds it to the CSV file for it to be read and print out "Tre is in house Dragon", etc. The code works; I'm just wondering if there's a way to shorten it.

import os
students = []
def get_name(student):
    return student["name"].capitalize()
while True:
    try:
        newname, newhouse = input("what is your name and house? ").split(',')
        file = open(r"C:\Users\Student\Documents\mywebsites\python projects\students.csv",'a')
        file.write(f"\n{newname},{newhouse}")
        file.close()
    except EOFError:
        break
    except ValueError:
         print("Please add a comma!")

with open(r"C:\Users\Student\Documents\mywebsites\python projects\students.csv") as file:
                for line in file:
                    names, house = line.rstrip().split(',')
                    student = {'name': names, 'house': house }
                    students.append(student)

for student in sorted(students, key=get_name):
        print(f"{student['name']} is in {student['house']}")
\$\endgroup\$
1
  • \$\begingroup\$ I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Commented Aug 7 at 18:39

5 Answers 5

9
\$\begingroup\$

DRY

You should store the long file path into a variable instead of replicating it multiple times:

file_path = r'C:\Users\Student\Documents\mywebsites\python projects\students.csv'
...
file = open(file_path, 'a')

Input prompt

It would be helpful to the user to know that the input requires a comma at the time when the input is requested. Put that request directly into the input string:

    newname, newhouse = input("what is your name and house (as name,house)? ").split(',')

Unused code

Since the code does not use os, this line can be deleted:

import os

Layout

There should be blanks lines before and after function definitions:

students = []
file_path = 'student.csv'

def get_name(student):
    return student["name"].capitalize()

while True:

There is also inconsistent indentation.
The black program can be used to automatically reformat your code.

Documentation

When I run the code, it is not clear to me how to stop entering input. You could create a header docstring to describe the purpose of the code as well as user instructions.

"""
Update the student.csv file to add more names and houses.
When done entering names, hit the blah-blah-blah key...

"""

Naming

Since these variables are localized to the while loop:

newname, newhouse

you can shorten them to:

name, house

names should be singular (name) since it only represents one name, not a list of names.

\$\endgroup\$
6
\$\begingroup\$
for line in file:
    names, house = line.rstrip().split(',')

This errors on my end:

Traceback (most recent call last):
  File "main.py", line 18, in <module>
    names, house = line.rstrip().split(',')
    ^^^^^^^^^^^^
ValueError: not enough values to unpack (expected 2, got 1)

That's because you use a leading newline:

file.write(f"\n{newname},{newhouse}")

So the csv starts with an empty line:


Tre,Dragon
Foo,Bar
Lorem,Ipsum

There are multiple ways to fix this, but here the simplest would be to skip empty lines when reading the students, e.g.:

for line in file:
    if line.strip():
        names, house = line.rstrip().split(',')
\$\endgroup\$
0
5
\$\begingroup\$

I agree with several suggestions from @toolic. For shortening:

  • Make r"C:\Users\Student\Documents\mywebsites\python projects\students.csv" a variable so you don't have to type it twice.
  • Remove import os because it is not used.
  • Remove extra indentation and inline variables such as file.
students = []
file_path = r"C:\Users\Student\Documents\mywebsites\python projects\students.csv"
def get_name(student):
    return student["name"].capitalize()
while True:
    try:
        newname, newhouse = input("what is your name and house? ").split(',')
        file = open(file_path, 'a')
        file.write(f"\n{newname},{newhouse}")
        file.close()
    except EOFError:
        break
    except ValueError:
         print("Please add a comma!")

for line in open(file_path):
    names, house = line.rstrip().split(',')
    students.append({'name': names, 'house': house})

for student in sorted(students, key=get_name):
    print(f"{student['name']} is in {student['house']}")

Some other suggestions:

  • Be careful with naming. The output of open is not a file, it is a stream. For descriptiveness, I would change file to stream and names to name. This lets us call file_path what it actually is: a file. Also, newname and newhouse could be renamed to name and house.
  • Declare variables close to their first use. I would move down students = [].
  • After the file is opened for the last time, the stream is never closed. This is usually not an issue, but I would explicitly close the stream. with is the way to go.
  • The instructions to the user could be more precise. In particular, I would use a special phrase like 'c' instead of EOFError to break out of the while loop. Since the special phrase has no comma, it will never be mistaken for a name,house input.
  • When you write to the file, put newlines at the end of lines, not the start.
  • Use whitespace to improve readability.
def get_name(student):
    return student["name"].capitalize()

file = r"C:\Users\Student\Documents\mywebsites\python projects\students.csv"
while True:
    print("To continue, type 'c' and press enter.")
    print("To enter your name and house, type '(name),(house)' and press enter.")
    user_input = input("Note: trailing whitespace is removed.")
    if user_input == 'c':
        break
    try:
        name, house = user_input.split(',')
        stream = open(file, 'a')
        stream.write(f"{name},{house}\n")
        stream.close()
    except ValueError:
        print("Please add a comma!")

students = []
with open(file) as stream:
    for line in stream:
        name, house = line.rstrip().split(',')
        students.append({'name': name, 'house': house})

for student in sorted(students, key=get_name):
    print(f"{student['name']} is in {student['house']}")

Hope that helps!

\$\endgroup\$
3
  • 2
    \$\begingroup\$ It looks safe enough, but with context managers should be preferred to manual stream.close() calls almost always - there's no harm in doing that, and such approach is more obviously correct. \$\endgroup\$
    – STerliakov
    Commented Aug 7 at 22:21
  • \$\begingroup\$ Thank you for the comment - taught me something new about with. \$\endgroup\$ Commented Aug 8 at 1:59
  • 2
    \$\begingroup\$ One reason we prefer to use a context manager is that they are robust to exceptions. If beneath the “with” statement there is a big block of code, or a function call into a big block of code, and some of that code is buggy, then a fatal uncaught exception might be raised. However, the context manager has its own try catch handler, which ensures that we shall certainly call close(), even if an unexpected exception happened. \$\endgroup\$
    – J_H
    Commented Aug 8 at 13:44
2
\$\begingroup\$

I have just a few remarks to add to what has already been said.

Restructure Your Loop to Minimize File Opens and try/except Executions

Currently your structure is:

while True:
    ...
    try:
        ...
        stream = open(file, 'a')
        ...
    except ...
    ...

For each new row you wish to append to the CSV file, you are establishing a try/except block and performing an open call. More efficient would be to re-structure your loop as:

stream = open('file', a)
try:
    while True:
        ...
except ...:

In that way you are opening the output file only once and establishing a try/except only once. This structure does not allow you to catch a missing comma the way you are currently doing it by causing a ValueError exception when unpacking the split line. By the way, the error message you generate assumes that no comma has been entered when it is equally possible that the user entered more than one comma. There are better ways to validate the input than relying on this method, as we shall see later.

Terminate Input with an Empty Line

Currently, you are expecting the user to terminate entering name/home pairs by signaling EOF. How to do that is platform-dependent and it is questionable that a non-technical user would know how to do this on any platform. Better to request that the user terminate input with an empty line.

Be More Flexible With What May Be Entered

The user might inadvertently enter additional spaces such as ' Booboo , My House '. You would want to strip leading and trailing whitespace on both the name and house.

The user might want to specify the name using a comma (e.g. 'John, Jr.') or the house with the actual mailing address that has one or more commas (e.g. '1 Main Street, New York'). Using a comma to separate the name from the house disallows these possibilities. One solution is to input the name and house individually with two input statements. Now there is no reliance on there being exactly one comma entered and a ValueError raised if this condition is not met.

Alternatively, to support commas as part of name or house, you could do a single input using a different separator, e.g. '|', which cannot be legally used in a value. Then you split the line and only when you have verified that the result is a list of length two do you do the unpacking. This will help you to generate a more meaningful error message.

But you still have the problem of correctly writing a new row to the CSV file so that commas within a value are not mistaken for additional columns.

One solution that easily allows you to have commas embedded within a value is to use the csv module, which takes care of outputting the necessary double-quotes around a column when necessary.

Putting It All Together

This is one possibility incorporating these ideas:

import csv

def get_name(student):
    return student["name"].capitalize()

CSV_FILE = r"C:\Users\Student\Documents\mywebsites\python projects\students.csv"

with open(CSV_FILE, 'a', newline='') as file:
    wtr = csv.writer(file)

    while True:
        name = input(
            "Enter your name or an empty line when done: "
            ).strip()
        if not name:
            break
        house = input(
            "Enter your house or an empty line when done: "
            ).strip()
        if not house:
            break
        wtr.writerow([name, house])

students = []
with open(CSV_FILE, newline='') as file:
    rdr = csv.reader(file)

    for row in rdr:
        student = {'name': row[0], 'house': row[1] }
        students.append(student)

for student in sorted(students, key=get_name):
    print(f"{student['name']} is in {student['house']}")
\$\endgroup\$
1
\$\begingroup\$

Let me address the most obvious issue: not using the csv standard Python library. You should use it because:

  • It handles the CSV data
  • It makes code easier to read
  • It has been well tested
  • It handles adding blank lines so you don't have to

Sample write:

import csv
# ...
with open(..., "a") as stream:
    writer = csv.writer(stream)
    writer.writerow([newname, newhouse])

Sample read:

import csv

def by_name(row: list):
    return row[0].lower(), row[1]

with open('data.csv') as stream:
    reader = csv.reader(stream)
    for row in sorted(reader, key=by_name):
        print("{} is in {}".format(*row))

As for other issues, other reviewers already cover them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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