BigBigBigWood / Backup

0 stars 0 forks source link

Review #1

Open Ctesias opened 7 years ago

Ctesias commented 7 years ago
  1. A few lines were too long if you wanted to conform to Pep8

  2. some tkinter frames do not have a .title property set you you end up with a title of tk,tk1, tk2, etc.. .title("My Application")

  3. The way the code is written it accepts a blank user/password since the admin lists are empty

  4. Password field is not masking input.

  5. I don't see any input sanitization on usernames/passwords

  6. During account creation the IF statements check to see if user, if admin, else message. There is no handling both boxes being selected. Should be an error or ask which is correct, or not allow to proceed, etc..

  7. there are no menu's created for the frames/widgets so we get default TCL/TKinter menus.

  8. Use descriptive names for your widgets, labels, etc... checkbox2 <-- this doesn't clue me in to what the check box is for. Try something like checkbox_user

##################################################################

  1. user_accounts = ['user'] user_passwords = ['password'] if entrybox1.get() in user_accounts and entrybox2.get() in user_passwords: so the username can be any user name in the list and the password can match any password in the list so I could type bob as the user and type jims' password as long as both are in the list I can log in At minimum you should be checking the user name, getting the index, then checking PW at that index assuming they were entered/appended in the same order.

###################################################################

  1. the function add_user() is broken. If I select both check boxes (USER, ADMIN) the first if will always execute and a User account will be created. There is no error handling or check to make sure both boxes aren't selected.

  2. The if checks also allow you to select a check box hit the create account button with no name or PW without throwing an error message or helpful message

  3. Needs to be more error handling and conditional checks.

  4. May be better to make the outter -- if len(entrybox3.get()) >= 1 and len(entrybox4.get()) >= 6: and inner ifs check box related. Some redundant code in this block

###################################################################

  1. Admin User list and Admin password list are both blank, allowing anonymous logins.
BigBigBigWood commented 7 years ago

Thanks for the diligent review. I am still working out the kinks since I have learned a hybrid of two versions of tkinter/TCL. Some of my code will need to be updated after some of the logic bugs have been ironed out.