2
\$\begingroup\$

I'm a beginner in Python programming. Currently, I learn how to work on the files... I've created a program, which excludes the names (first & last) from each row, that contains both of them. When the pair of names does not include for example the Last Name part, it places n/a in the last_names.txt file.

Like from the file names.txt:

Paul Morgan
James Kowalski
Thomas

sort to two files

firstnames.txt:

Paul
James
Thomas

lastnames.txt:

Morgan
Kowalski
n/a
"""
Program separating first names and last names gathered in pairs from one file and saving them into two files.
"""

"Global Variables:"
allNamesFromFile = []


def names_separator(fileName):
    with open(fileName, "r+", encoding="utf-8-sig") as namesFromFile:
        for names in namesFromFile:
            allNamesFromFile.append(tuple(names.replace("\n", "").split(" ")))
        open("last_names.txt", "w").close() #cleaning the last_names.txt file
        open("first_names.txt", "w").close() #cleaning the first_names.txt file
        lastNamesFile = open("last_names.txt", "a+", encoding="utf-8-sig")
        firstNamesFile = open("first_names.txt", "a+", encoding="utf-8-sig")
        for namesTuples in allNamesFromFile:
            try:
                lastNamesFile.write(namesTuples[1] + "\n")
            except:
                lastNamesFile.write("n/a\n")
            try:
                firstNamesFile.write(namesTuples[0] + "\n")
            except:
                firstNamesFile.write("n/a\n")
    lastNamesFile.close()
    firstNamesFile.close()


print("welcome in the names separator program...")
while True:
    fileName = str(input("Please provide the file name with its extension: "))
    try:
        names_separator(fileName)
        print("operation completed")
        break
    except:
        print("You have provided an incorrect file name...")
        continue

I wonder if there is anything I could improve in this code... I wanted to use list + tuples. The program works correctly, but I want to review it and maybe improve if it's possible. Maybe I could write it shorter/better, or just its functionality can be optimized?

Thanks in advance.

\$\endgroup\$
5
  • \$\begingroup\$ Welcome to Code Review! Did you recently post this same question here on Code Review or elsewhere on the Stack Exchange network? It looks very familiar. \$\endgroup\$
    – Mast
    Commented Feb 6, 2020 at 7:43
  • 3
    \$\begingroup\$ @Mast Hey Mast. Thanks for the welcome :) Yes I did. I put that initially on the StackOverFlow, but they told me to put it here... So I've deleted it from there and placed it here \$\endgroup\$ Commented Feb 6, 2020 at 8:47
  • \$\begingroup\$ Are names guaranteed to have at most two components? In other words, are there no middle names or two name last names? \$\endgroup\$
    – Graipher
    Commented Feb 6, 2020 at 11:30
  • \$\begingroup\$ @Graipher Hey. Yes. That was the assignment description. In this code, I just focus only on the first and last names. Considering there are no middle or two names \$\endgroup\$ Commented Feb 7, 2020 at 12:27
  • \$\begingroup\$ Can you clarify what the program is meant to do? \$\endgroup\$
    – AMC
    Commented Feb 8, 2020 at 22:29

1 Answer 1

3
\$\begingroup\$

The functionality of your program seems slightly weird. You read multiple files, and each one gets split, but in doing so you (explicitly) delete the old data, is this right?

One thing I notice is this piece of code here:

open("last_names.txt", "w").close() #cleaning the last_names.txt file
open("first_names.txt", "w").close() #cleaning the first_names.txt file
lastNamesFile = open("last_names.txt", "a+", encoding="utf-8-sig")
firstNamesFile = open("first_names.txt", "a+", encoding="utf-8-sig")

This could be replaced by:

lastNamesFile = open("last_names.txt", "w", encoding="utf-8-sig")
firstNamesFile = open("first_names.txt", "w", encoding="utf-8-sig")

I'd also move it to the start of your function:

with open(fileName, "r", encoding="utf-8-sig") as namesFromFile, 
     open("last_names.txt", "w", encoding="utf-8-sig") as lastNamesFile,
     open("first_names.txt", "w", encoding="utf-8-sig") as firstNamesFile:

So you can remove the close statements at the end. I'm not sure what the encoding= does, but it might not be necessary.

You can also reduce this:

    for names in namesFromFile:
        allNamesFromFile.append(tuple(names.replace("\n", "").split(" ")))

    for namesTuples in allNamesFromFile:
        try:
            lastNamesFile.write(namesTuples[1] + "\n")
        except:
            lastNamesFile.write("n/a\n")
        try:
            firstNamesFile.write(namesTuples[0] + "\n")
        except:
            firstNamesFile.write("n/a\n")

To this:

for names in namesFromFile:
    names_split = tuple(names.replace("\n", "").split(" "))
    if len(names_split) >= 1:
        firstNamesFile.write(names_split[0] + "\n")
    else:
        firstNamesFile.write("n/a\n")

    if len(names_split) >= 2:
        lastNamesFile.write(namesTuples[1] + "\n")
    else:
        lastNamesFile.write("n/a\n")

Allowing a for loop less and two less try ... except blocks as well as a global list which can now be left out:

"Global Variables:"
allNamesFromFile = []

I don't think this is necessary:

fileName = str(input("Please provide the file name with its extension: "))

Instead use:

fileName = input("Please provide the file name with its extension: ")

I'd also look at this part:

try:
    names_separator(fileName)
    print("operation completed")
    break
except:
    print("You have provided an incorrect file name...")
    continue

You are catching a lot of exceptions, try to be more precise. Instead try:

except FileNotFoundError:

Lastly, you might want to check naming conventions, variables should be in snake_case and classes in CamelCase.

So, to summarize:

  • check if your program does what you want it to do
  • open files using with
  • limit the amount of global variables to 0
  • limit the amount of try ... except blocks
  • be explicit in the exceptions you're catching
  • check pep8 style guide for naming of variables
\$\endgroup\$
1
  • \$\begingroup\$ Hey! I really appreciate your answer. You are right. Now when I look at my code, it makes no sense to have so many tries/excepts. Also, the for loop you have used in your advice is much better. I've changed my code and it works, and I understand those changes. I use encoding UTF-8 to keep polish "ź, ż, ś, etc." letters. Thanks a lot! \$\endgroup\$ Commented Feb 11, 2020 at 8:54

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.