Python Forum
Criticism on one of my first completed programs in Python
Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Criticism on one of my first completed programs in Python
#2
I didn't run your program, because of all of the absolute paths. Relative paths are better if you can manage them. Otherwise note that you can generate absolute paths to the program file dynamically (os.path.abspath(__file__)), and then add relative paths onto that to get at specific files. Then you don't have to hunt down and change file paths when moving your programs around.

Your variables names confuse me, because you are using a lot of abbreviations. Use smaller words instead. Like genAcctNum. I'm assuming that's short for generateAccountNumber. I would just say getAccountNum. Actually, I would say get_account_num, and some people might get on your case because the camel case is not PEP8 compliant under their interpretation.

Your check for existing account numbers is very odd and complicated. Why not:

acctNumFile = open(fileLocation)
acctNums = [line.strip for line in accNumFile]
while True:
    newNum = str(random.randint(100000000000, 999999999999))
    if newNum not in acctNums:
        break
You are using a lot of string addition. The format method or f-strings would be better.

You named a variable file. 'file' is a keyword in Python, don't use it for a variable name.

I'm not one to normally harsh on the use of eval, but you are using it to parse numbers out of a string, which is just lazy. [int(word) for word in text.split()] works fine.

You have an open except clause, which will get unexpected errors as well as expected ones. Figure out what your expected errors are, and catch those specifically. Not using eval to parse numbers from a string will help lower the number of expected errors.

This code does nothing:

else:
    pass
If there is no else clause, and none of the conditionals are met, it will just keep processing. Which is what that code takes up two lines to do.

You have the code not(PIN.isdigit). That is never true, because you are referring to the method isdigit, not calling the method isdigit. Also, not is not a function, it is an operator. So don't put the parens there unless there is a logical expression in them that needs scope clarification. If you do use parens, put a space before them so it doesn't look like a function call.

You appear to be trying to use dunders and getters to control variable access. That doesn't actually do anything. Look into properties if you really want something like that.
Craig "Ichabod" O'Brien - xenomind.com
I wish you happiness.
Recommended Tutorials: BBCode, functions, classes, text adventures
Reply


Messages In This Thread
RE: Criticism on one of my first completed programs in Python - by ichabod801 - Jul-05-2018, 07:26 PM

Possibly Related Threads…
Thread Author Replies Views Last Post
  Tkinter Tic Tac Toe With Enhanced Features - Completed adt 2 2,935 Dec-10-2019, 06:22 AM
Last Post: adt
  my earliest completed script Skaperen 0 1,998 Mar-08-2019, 09:50 PM
Last Post: Skaperen
  [link]Creating-a-repo-for-your-completed-scripts metulburr 0 7,616 Aug-29-2018, 01:19 PM
Last Post: metulburr
  completed module: validip.py Skaperen 8 7,398 Jan-04-2017, 06:39 AM
Last Post: Skaperen

Forum Jump:

User Panel Messages

Announcements
Announcement #1 8/1/2020
Announcement #2 8/2/2020
Announcement #3 8/6/2020