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']}")
Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Aug 7, 2024 at 17:42
\$\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, 2024 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.

J_H
41.4k3 gold badges38 silver badges157 bronze badges
answered Aug 7, 2024 at 18:17
\$\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(',')
answered Aug 7, 2024 at 19:09
\$\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!

answered Aug 7, 2024 at 19:09
\$\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\$ Commented Aug 7, 2024 at 22:21
  • \$\begingroup\$ Thank you for the comment - taught me something new about with. \$\endgroup\$ Commented Aug 8, 2024 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\$ Commented Aug 8, 2024 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']}")
answered Aug 10, 2024 at 20:14
\$\endgroup\$
2
\$\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.

answered Aug 27, 2024 at 20:26
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.