4
\$\begingroup\$

This is my project(beta) after studying for 3 weeks. It took me more or less 30 hours of writing and researching. I would appreciate a review based on my beginner level. I'm very proud of it, it was hard work for me.

Learned... Files management, control if for while loops, integers and strings, lists dictionaries and tuples.

Code:

#C:\restaurant software
with open('names.txt', 'r') as r :
    f_n = r.read().splitlines()
print("Welcome to NAME.app")
##############
# USER LOGIN #
##############
while True:
    name = input("""
    \n - Insert name to logg in
    \n - ADD to save new user
    \n - LIST to see saved users
    \n - REMOVE to delete a user
    \n - EXIT to finish
    \n - ...""")

    lname = name.lower()

    if lname == "add":
        n_input = input("Name:")
        with open('names.txt', 'a') as f:
            f.write(n_input + '\n')

    elif lname == "list":
        with open('names.txt') as f:
            print(f.read().splitlines())
            f.close()

    elif name in f_n:
        print("Logged as", name.upper())
        user = name
        input('Welcome, press enter to continue \n')
        break

    elif lname == 'remove':
        rem = input("Insert user name to remove \n ...")
        with open('names.txt', 'r+') as f:
            l = f.readlines()
            l = [z for z in l if rem not in z]
        with open('names.txt', 'w') as f:
            f.writelines(l)

    elif lname == "exit":
        exit()
####################
# TABLE MANAGEMENT #
####################
#C:\restaurant software
while True:
    action = input ('''
 - NEW table
    \n - ADD table
    \n - BILL
    \n - CLOSING
    \n - EXIT
    \n - ... ''')

    d = {'(1) chburger': 19,'(2) bncburger': 23,'(3) plpasta': 6}

    if action == 'new' :
        tn = input('Insert table number \n - ...')
        name = 'T' + tn
        t = open(name + '.txt', 'w+')
        print('Done')

    elif action.lower() == 'add':

# Select table
        table = input ('Select desired table number: \n - ...')
        fulltab = 'T' + table
        with open(fulltab + '.txt', 'w+') as f :

# Order list and add Order
            while True:
                for k, v in d.items() :
                    print(k, v)
                addprod = input('Insert order. \n - ...')
                for k, v in d.items() :
                    if addprod == k[1] :
                        f.write(str(v) + '\n')

#Option to continue.
                q = input('Add more? y/n \n -...')
                if q.lower() == 'y' : continue
                if q.lower() == 'n' : break

 #File as F

    elif action.lower() == 'bill' :
        p_b = input('Please insert number of table. \n -... ')
        with open (('T' + p_b)+ '.txt', 'r+') as p :
            tobill = 0
            for line in p : tobill = int(tobill) + int(line)

            xtra = input('Group table (+10 ppl)? y/n: \n')
            if xtra == 'y' :
                tobill = tobill + (tobill/100)*10
                print('SERVICE CHARGE ADDED.')

            elif xtra == 'n' : print ('Processing bill...')
            print('Total to pay:', tobill)
            print('Serviced by', user)

            #### Closing added part to bill.
            with open('closing.txt', 'a+') as f :
                f.write(str(tobill) + '\n')

# Closing days balance.
    elif action.lower() == 'closing' :
        date = input('Please enter DATE. ')
        with open('closing.txt', 'r+') as f :
            result = 0
            for line in f : result = int(result) + int(line)
            print('Closing is...')
            print(result)
            print('Today tips', (result / 100 * 10))
        current_closing = 'C:restaurant software\\closing.txt'
        old_closing = 'C:restaurant software\\' + date + '.txt'
        import os
        os.rename(current_closing, old_closing)
        print('Day closed, find the closing balance in the specific dated document.')
# Exit command.
    elif action.lower() == 'exit' :
        exit()
 

I know I have to improve the log in system with the possible names that someone could enter and a lot more of things.

Before starting this project the largest that I write was 30 lines.

\$\endgroup\$
4
\$\begingroup\$

Functions

Study functions. Your code would greatly benefit from breaking up into functions.

Bugs

User Management

  • If you add the name "Anne", you cannot login with the name "Anne" until you quit and restart the program.
  • If you remove the name "Anne", you can still login with the name "Anne" until you quit and restart the program.
  • If you remove the name "Anne", it will also remove the name "Mary-Anne".
\$\endgroup\$
3
\$\begingroup\$

File storage

Text files are not convenient for lookups. So your next logical step should be to learn about databases and SQL. With Python you can use SQLite to build a small, self-contained database file.

Security

Entering a user name is not enough, there should be a password too. I am assuming this is for a family business or a small circle of very trusting people. Speaking of accountability, I would advise that you get familiar with the Python logging module. I would log all meaningful events to a file. For example, when booking a table, it is interesting to know who carried out the action and when. Then a single line like this would result in a timestamped entry into the log file:

logger.info(f"Table #{table_number} booked by user {username}")

Same when cashing in money, this type of event should be recorded (who-what-when), preferably not in a place that can easily be tampered with, even if I understand you are not a bank.

Don't repeat yourself (DRY)

There is a lot of repetition eg with open('names.txt', 'r+') as f:. The file name (and path) should be declared once as a 'constant' variable at the top of the script. Then break up your code in small functions. For example a function to check if a user name is valid - it has to return True or False. Each menu entry should call a function, to better separate tasks and to keep the code as small and lean as possible.

The files are surely very small, but to optimize things, you could load the lists once, when starting up your program. Reload them only if changes occur.

User input

Validation of user input is very important or your program will crash or behave predictably. That may involve regular expressions depending on the patterns you need to validate.

Coding style

Imports like import os should be at the top of the script, not declared just before using some function. Get familiar with PEP8 coding rules. This: with open('closing.txt', 'a+') as f : should be written: with open('closing.txt', 'a+') as f:. You are doing it right in some parts of the code but not everywhere. Consistency is important and PEP8 is considered the Bible of Python programmers.

I suggest to use an editor with a linter that will help enforce those rules and highlight problematic lines.

It is good that you are using context managers with files, but then you don't need to explicitly close the file here:

elif lname == "list":
    with open('names.txt') as f:
        print(f.read().splitlines())
        f.close()

...as long as you stick with this approach, which is the recommended way and not just for files.

Give more thoughts about variable naming. For example in this code the name f_n is not intuitive:

elif name in f_n:
    print("Logged as", name.upper())
    user = name
    input('Welcome, press enter to continue \n')
    break

It should be user_names, valid_users or something like that. Then you can write:

if user in valid_users:
    # ...

Which makes the code semantically more clear, more intuitive and easier to read.

Another selected snippet:

p_b = input('Please insert number of table. \n -... ')
with open (('T' + p_b)+ '.txt', 'r+') as p :

what does p_b mean ? Pay the bill I suppose... Fortunately the variable is defined just above and I don't have to scroll 500 lines of code to find out what it means. Seems to me that p_b should be named table_number simply. There is no penalty for using longer and more meaningful variable names. Only benefits.

For loop should be formatted like this:

for line in p:
    tobill = int(tobill) + int(line)

instead of:

for line in p : tobill = int(tobill) + int(line)

Another more cryptic line:

l = [z for z in l if rem not in z]

Without analyzing your code it's not immediately obvious what this line is doing. Even if it's clear to you now, it won't be when you revisit your code after a few months.

To further comment on the offending code:

elif lname == 'remove':
    rem = input("Insert user name to remove \n ...")
    with open('names.txt', 'r+') as f:
        l = f.readlines()
        l = [z for z in l if rem not in z]
    with open('names.txt', 'w') as f:
        f.writelines(l)

First of all, you are rebuilding the list of users every time. The performance hit is negligible until you work with large files. Clearly, using files has its limits for what you are doing. But the approach is unsafe, you could destroy the file if an exception occurs in this code section. And exceptions related to file I/O are pretty common, between permissions issues, lack of disk space or path errors. One more reason for using a database (with transactions).

Imagine that you make a mistake in the file name when reading it, and accidentally read from another file that happens to exist. What do you think the ultimate result would be when you write to names.txt ? That's one more reason for not repeating yourself. Define file names and paths as constants once for all. At some point you may have to change the file name or path, and it's better to change it at one place only, rather than review all the code manually, at the risk of missing out one line, or do a search and replace that may be incomplete.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Wow! I suppose this will take me a good few days to fix. And I'm going straight to learn about SQL. Thank you very much for the time you spent writing this, it really helps me. \$\endgroup\$ Dec 31 '20 at 10:04

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.