21
\$\begingroup\$

This code is for an employee management system. The goal that I am trying to reach is to allow managers to access data of their employees more easily. The data that is entered is saved to a CSV file and I have systems in place that will allow those managers to access their employees' data through a quick search of the file. I'm 16 and a beginner, please explain some good clean code concepts that you guys recommend.

''' Employee System By Ronald Colyar : 1/2/2018 ''' #our modules for this project import csv from tkinter import * from tkinter import ttk #main class gui class employee_gui : def __init__(self , master): self.master = master #configuring the title of the main window , aswell as the background color master.title(string = 'Pha<n>tex Employee Management System') master.configure(background = 'black') #employee first name self.firstname_label = Label(master, text = 'Employee First Name ***' ,bg= 'black', fg ='white') self.firstname_label.grid(row = 1 , column = 0 , sticky ='we', padx=5, pady=5) self.first_name = ttk.Entry(master) self.first_name.grid(row = 2 ,column = 0 , sticky = 'we',padx=5, pady=5 ) #employee lastname self.lastname_label = Label(master, text = 'Employee last Name ***',bg= 'black', fg ='white') self.lastname_label.grid(row = 1 , column = 1 , sticky ='we',padx=5, pady=5) self.last_name = ttk.Entry(master) self.last_name.grid(row = 2 ,column = 1 , sticky = 'we',padx=5, pady=5) #employee email self.employee_email = Label(master, text = 'Employee Email ***',bg= 'black', fg ='white') self.employee_email.grid(row = 3 , column = 0 , columnspan = 2 , sticky = 'we') self.employee_email_entry = ttk.Entry(master) self.employee_email_entry.grid(row = 4 , column = 0 , columnspan = 3 , sticky = 'we', padx=5, pady=5) #day self.DAY= Label(master, text = 'Day**',bg= 'black', fg ='white') self.DAY.grid(row = 5 , column = 0 , sticky = 'we',padx=5, pady=5) #day options self.dayoptions = ['1', '2' , '3' , '4' , '5' , '6' , '7' , '8' , '9', '10' , '11' , '12' , '13' , '14' , '15' , '16' , '17' , '18' , '19', '20' , '21' , '22' , '23' , '24' , '25' , '26' , '27' , '28' , '29' , '30' , '31'] #day container self.dayvar= StringVar() self.dayvar.set('none') #day option menu self.dropdown_day = ttk.OptionMenu(master,self.dayvar , *self.dayoptions) self.dropdown_day.grid(row = 6 , column = 0 , sticky= 'we') #month self.month= Label(master, text = 'month**',bg= 'black', fg ='white') self.month.grid(row = 5 , column = 1 , sticky = 'we',padx=5, pady=5) #month options self.monthoptions = ['1' , '2' , '3' , '4' , '5' , '6' , '7' , '8' , '9' , '10' , '11' , '12'] #month container self.monthvar = StringVar() self.monthvar.set('select') #MONTH DROPDOWN MENU self.dropdown_month = ttk.OptionMenu(master, self.monthvar , *self.monthoptions) self.dropdown_month.grid(row = 6 , column = 1 , sticky = 'we',padx=5, pady=5) # year #year options self.years_unsplit = '2029 - 2028 - 2027 - 2026 - 2025 - 2024 - 2023 - 2022 - 2021 -2020 - 2019 - 2018 - 2017 - 2016 - 2015 - 2014 - 2013 - 2012 - 2011 -2010 - 2009 - 2008 - 2007 - 2006 - 2005 - 2004 - 2003 - 2002 - 2001 -2000 - 1999 - 1998 - 1997 - 1996 - 1995 - 1994 - 1993 - 1992 - 1991 - 1990 - 1989 - 1988 - 1987 - 1986 - 1985 - 1984 - 1983 - 1982 - 1981 -1980 - 1979 - 1978 - 1977 - 1976 - 1975 - 1974 - 1973 - 1972 - 1971 -1970 - 1969 - 1968 - 1967 - 1966 - 1965 - 1964 - 1963 - 1962 - 1961 -1960 - 1959 - 1958 - 1957 - 1956 - 1955 - 1954 - 1953 - 1952 - 1951 -1950 - 1949 - 1948 - 1947 - 1946 - 1945 - 1944 - 1943 - 1942 - 1941 -1940 - 1939 - 1938 - 1937 - 1936 - 1935 - 1934 - 1933 - 1932 - 1931 -1930 - 1929 - 1928 - 1927 - 1926 - 1925 - 1924 - 1923 - 1922 - 1921' self.yearoptions2 = self.years_unsplit.split('-') #year variable self.yearvar = StringVar() self.dropdown_year = ttk.OptionMenu(master , self.yearvar , *self.yearoptions2) self.dropdown_year.grid(row = 6 ,column = 2 , sticky = 'we') self.year = Label(master, text = 'Year**' ,bg= 'black', fg ='white') self.year.grid(row = 5 , column = 2, sticky = 'we', padx=5, pady=5) #the address section self.Address = Label(master ,text = 'Address(optional)*** Example: 110 s. grove street',bg= 'black', fg ='white') self.Address.grid(row = 7 , column = 0 ,sticky = 'we',padx=5, pady=5) self.Address_entry = ttk.Entry(master) self.Address_entry.grid(row = 8 , column = 0 , columnspan = 3, sticky = 'we',padx=5, pady=5) #the position_ occupation section self.Position = Label(master , text= 'Position/Occupation***' , bg= 'black', fg ='white') self.Position.grid(row = 9 , column = 0 , sticky = 'we',padx=5, pady=5) self.Position_entry = ttk.Entry(master) self.Position_entry.grid(row = 10 , column = 0 ,columnspan = 3, sticky = 'we',padx=5, pady=5) #thesalary section self.salary = Label(master , text = 'Employee Salary***',bg= 'black', fg ='white') self.salary.grid(row = 11 , column = 0 , sticky = 'we',padx=5, pady=5) self.salary_entry = ttk.Entry(master) self.salary_entry.grid(row = 12 , column = 0 , sticky = 'we',padx=5, pady=5) #the employee id informaiton self.employee_id_label = Label(master, text = 'Employee Id **VERY IMPORTANT**',bg= 'black', fg ='white') self.employee_id_label.grid(row = 13 , column = 0 , sticky = 'we',padx=5, pady=5) self.employee_id_entry = ttk.Entry(master) self.employee_id_entry.grid(row = 14 , column = 0 , sticky = 'we' ,padx=5, pady=5 ) #the main menu bar self.titlebaroptions = Menu(master) #the file section in the menu bar self.filesystem = Menu(master,tearoff=False) self.filesystem.add_command(label = 'Display Employee Information' , command =display_information_window) self.savesystem = Menu(master,tearoff=False) self.savesystem.add_command(label= 'Save Employee File' , command= save_information_window) self.deletesystem =Menu(master,tearoff=False) self.deletesystem.add_command(label = 'Delete Employee' , command = delete_information_window) self.help = Menu(master, tearoff = False) self.help.add_command(label= 'Help' , command = help_window) #adding the sections to the main menu bar self.titlebaroptions.add_cascade(label = 'Open' , menu = self.filesystem) self.titlebaroptions.add_cascade(label = 'Save' , menu = self.savesystem) self.titlebaroptions.add_cascade(label = 'Remove Employee' , menu =self.deletesystem) self.titlebaroptions.add_cascade(label = 'Help' , menu = self.help) # adding main menu to the master window master.config(menu = self.titlebaroptions) #the icon for the window master.iconbitmap('employeeicon.ico') #logos self.photo = PhotoImage(file = 'phantexlogo.png') self.phantexlogo = Label(master, image = self.photo, bg= 'black', fg ='white') self.phantexlogo.image = self.photo self.phantexlogo.grid(row = 15 , column = 0 , sticky = 'we') self.photo2 = PhotoImage(file = 'phantexlogo.png') self.phantexlogo2 = Label(master, image = self.photo, bg= 'black', fg ='white') self.phantexlogo2.image = self.photo self.phantexlogo2.grid(row = 15 , column = 1 , sticky = 'we') def delete_information_method(): #a tkinter entry global delete_entry, csv_writer1 #opening csv in read with open('employees.csv' , 'r', newline='') as emp_read: #creating our dictreader csv_dictreader = csv.DictReader(emp_read) fieldnames = csv_dictreader.fieldnames contents = [line for line in csv_dictreader] #opening csv file in write mode with open ('employees.csv' , 'w', newline='') as emp_write: #creating our writer csv_writer1 = csv.DictWriter(emp_write, fieldnames=fieldnames) csv_writer1.writeheader() #our loop to check each line inside of our csv file is not equal to what is inside the delete entry for line in contents: if line['employee id'] != str(delete_entry.get()): csv_writer1.writerow(line) #grabbing all the employee data and inserting it inside of the listbox def all_emp_search(): global information_box #deleting data out of the listbox , that was previously there information_box.delete(0 , 'end') #our read file with open ('employees.csv' , 'r') as employee_read_file: #inserting all information inside of the csv file into the listbox for line in employee_read_file: information_box.insert(END , line) #grabbing a single employee data and inserting it inside the listbox def single_emp_search(): global search_label_entry,information_box #deleting data out of the listbox , that was previously there information_box.delete(0 , END) #our read file with open ('employees.csv' , 'r') as employee_read_file: ''' searching to see if the contents of the csv file matches what is put inside of the entry and if so inserting the line into the listbox ''' for line in employee_read_file: if line.find(str(search_label_entry.get())) > -1: information_box.insert(END , line) #the information deletion window def delete_information_window(): global delete_entry delete_frame = Toplevel() delete_frame.config(background = 'black') #intro label(title) delete_intro_header = Label(delete_frame , text = 'Welcome to the Delete section' , font = 'times 14 bold',bg= 'black', fg ='white') delete_intro_header.grid(row = 2 , column = 0 , sticky = 'we') #The entry label/header delete_entry_header = Message(delete_frame , text = 'Enter in the employee ID , you would like to remove , if you dont recall , you can access the employee information , by going to Mainpage/Open/Display Employee Information, here you can search for an employee name , and all the information including the ID will be present',bg= 'black', fg ='white') delete_entry_header.grid(row = 3, column = 0 , sticky= 'we') # delete entry delete_note_header = Label(delete_frame , text = 'Note: Once You Delete An Employee There is no recovery , be careful with this process',bg= 'black', fg ='white') delete_note_header.grid(row = 4, column = 0 , sticky = 'we') delete_entry = ttk.Entry(delete_frame ) delete_entry.grid(row = 5, column = 0 , sticky = 'we') #the delete button(submit button) delete_button = Button(delete_frame , text = 'Delete Employee' , fg = 'white', bg = 'black',command = delete_information_method) delete_button.grid(row = 5 , column = 1, sticky = 'we') #the information display window def display_information_window(): global search_label_entry,information_box display_frame = Toplevel() display_frame.config(background = 'black') #the intro message(title) intro_message = Label(display_frame, text = 'Welcome to the Display section' , font = 'times 14 bold',bg= 'black', fg ='white') #the header for the search_label_entry search_label = Label(display_frame , text = 'Search for one Employee information',bg= 'black', fg ='white') search_label.grid(row = 3 , column = 0 , sticky = 'we') #the search entry search_label_entry = ttk.Entry(display_frame) search_label_entry.grid(row = 4, column = 0 , sticky = 'we') all_information_label = Label(display_frame , text = 'All employee information',bg= 'black', fg ='white') all_information_label.grid(row = 3 , column =2 , sticky = 'we') #our listbox information_box = Listbox(display_frame, bd = 0 , width = 70) information_box.grid(row = 6 , column = 0, sticky= 'we') #all information button all_information_button = Button(display_frame , text = 'All Information',bg = 'black' , fg = 'white', command = all_emp_search) all_information_button.grid(row = 4 , column = 2 , sticky = 'we') #emp single search Button search_single_emp = Button(display_frame , text = 'Search Single Employee', command = single_emp_search,bg = 'black' , fg = 'white' ) search_single_emp.grid(row = 5 , column = 2 , sticky = 'we') #help window def help_window(): help_frame = Toplevel() intro_message = Label(help_frame, text = 'Welcome to the help section' , font = 'times 14 bold') intro_message.grid(row = 3 , column = 0 , sticky = 'we') mainmessage = Message(help_frame, text = 'The way this program works , is it allows you to store your employee information , and go back and later access it , using the employee ID feature. The Employee ID makes accessing your information more smooth and manageable , feel free to use this program for your buisnesses and ect. Try to keep the employee IDs different for management purposes') mainmessage.grid(row = 4 , column = 0 , sticky ='we') help_frame.iconbitmap('employeeicon.ico') #saving the information to the csv file method def save_information_window(): global csvwriter #grabbing all the information from our entrys first_info = mainmenu_submit.first_name.get() last_info = mainmenu_submit.last_name.get() email_info = mainmenu_submit.employee_email_entry.get() day_info = mainmenu_submit.dayvar.get() month_info = mainmenu_submit.monthvar.get() year_info = mainmenu_submit.yearvar.get() position_info = mainmenu_submit.Position_entry.get() employee_salary_info = mainmenu_submit.salary_entry.get() employee_id_info = mainmenu_submit.employee_id_entry.get() adress_info = mainmenu_submit.Address_entry.get() #putting our information into a list of strings , fieldnames_list = ['first name' , 'last name' , 'email' , 'DOB' ,'adress' ,'position' , 'employee salary' , 'employee id' ] whole_information = [str(first_info) , str(last_info) , str(email_info) , str(month_info + '-' +day_info +'-' + year_info ) ,str(adress_info), str(position_info) , str(employee_salary_info) , str(employee_id_info) ] #creating our dictonary , using the fieldnames as the key and the whole information as the value of the dictionary my_whole_info_dict = dict(zip(fieldnames_list , whole_information)) #opening our csv file in write mode and adding the data from the entrys with open('employees.csv', 'a' ,newline = "") as employee_file: csvwriter = csv.DictWriter(employee_file, fieldnames = fieldnames_list , delimiter = ',') csvwriter.writeheader() csvwriter.writerow(my_whole_info_dict) #main window root = Tk() #employee_gui object mainmenu_submit = employee_gui(root) #constant loop root.mainloop()
\$\endgroup\$
0

4 Answers 4

Reset to default
18
\$\begingroup\$

Welcome to Code Review! Kudos to writing a fairly large program.

Several things pop-out from your program. But, a few things first. If you are using any intelligent editor; please see if you can get a python linter (or a PEP-8 integration) in it. PEP-8 is the python's style guide, which makes the code consistent, and hence, readable/maintainable.

I noticed that you are creating months, days and years list. Use the range function. It'll generate the whole list for you. You can later map the list to convert each value to string (if tkinter expects strings):

self.yearoptions2 = map(str, range(2029, 1921, -1))

Do not rely on global variables. As your codebase grows; it'll be difficult to follow which globals are referencing what. Pass the delete_entry, information_box etc. values as parameters to your functions instead.

Nest the execution of your tkinter application inside an if __name__ block:

if __name__ == "__main__": #main window root = Tk() #employee_gui object mainmenu_submit = employee_gui(root) #constant loop root.mainloop()

I haven't executed your program yet. However, I'd also suggest that instead of maintaining CSV's for the purpose of database, you make use of an actual DBMS. Out of the box, python provides you with sqlite3 package.

This will help in performance when your employee records reach let's say a 1000 employees and you want to filter employees within a salary range or similar analytical observations.

Some important PEP-8 pointers you can follow:

  • A statement/line-of-code should be at most 80 characters
  • Avoid whitespaces in following scenarios:
    • Immediately before a comma, semicolon, or colon
    • Immediately before the open parenthesis that starts the argument list
  • Don't use spaces around the = sign when used to indicate a keyword argument
\$\endgroup\$
7
  • \$\begingroup\$ woah , you got some great points ,so you think sqlite3 would be better? , i use sublime text 3 as a enviorment btw \$\endgroup\$ Commented Jan 10, 2018 at 9:38
  • \$\begingroup\$ aswell as , do you really think an if name is main statement would be necessary , or are you making it future proof? \$\endgroup\$ Commented Jan 10, 2018 at 9:41
  • 1
    \$\begingroup\$ @RonComputing An if __name__ == "__main__": construct is necessary (besides looking a heck of a lot neater than not using it since it immediately tells the reader what the top level of your code is) when you start to import your code from elsewhere. If you don't do it, all code will be executed on import. With such a clause, it will only be executed if the file itself is executed instead of imported by another file. \$\endgroup\$ Commented Jan 10, 2018 at 9:44
  • 1
    \$\begingroup\$ @RonComputing packagecontrol.io/packages/Python%20PEP8%20Autoformat sqlite3 is just a suggestion. Maintaining CSV is hard, and inefficient for analytical purposes. Since you are a beginner, sqlite3 would be the easiest DBMS to learn. \$\endgroup\$ Commented Jan 10, 2018 at 9:48
  • 2
    \$\begingroup\$ @RonComputing just to talk a little bit about sqlite for a moment. In the "real world" most programs will be dealing with SQL in some way so most developers will need to know at least a little SQL. sqlite is the simplest SQL database to setup, it is much more lightweight than things like MySQL or Postgres. \$\endgroup\$ Commented Jan 10, 2018 at 14:22
14
\$\begingroup\$

Don't do wildcard imports

Use import tkinter as tk and then prefix all tk classes and commands with tk. (eg: tk.Tk(), tk.Frame(...), etc).

PEP8 discourages wildcard imports, and for good reasons. They pollute the global namespace, and they can overwrite variables and classes without you knowing. Tkinter is particularly susceptible to this since both tk and ttk define classes with the same name.

Use PEP8 naming conventions

You should adopt PEP8 naming conventions. Specifically, start class names with an uppercase character and use CamelCase (eg: class EmployeeGUI).

Don't add useless comments

Comments are important, but they can also be a source of noise. Comments like #employee first name immediately before creating a variable named self.firstname_label are pointless.

Group your layout statements together

You should separate widget creation from widget layout. By grouping your layout code together it's much easier to visualize, and much easier to modify. It's also easier to notice inconsitencies, such as using slightly different colors or paddings for some widgets. My experience tells me that during development the layout can change fairly often.

Organize them in the same groups as they appear in the UI. For example, if all of your labels and entries are in one big block in the UI, make them one big block in the code. If you break them out into separate UI sections (eg: an address block, a name block, etc.) then organize them that way.

For example:

 self.firstname_label = Label(...) self.first_name = ttk.Entry(...) self.lastname_label = Label(...) self.last_name = ttk.Entry(...) self.employee_email = Label(...) self.employee_email_entry = ttk.Entry(...) ... self.firstname_label.grid(....) self.first_name.grid(...) self.lastname_label.grid(...) self.last_name.grid(...) self.employee_email.grid(...) self.employee_email_entry.grid(...) ...

Whether you do "label, entry, label, entry, ...", or "label, label, label, ..., entry, entry, entry, ..." is whatever you think makes the code easier to understand.

\$\endgroup\$
5
  • \$\begingroup\$ great point i will look into organizing my code in groups seeing as though that will be eaiser to manage \$\endgroup\$ Commented Jan 10, 2018 at 12:22
  • \$\begingroup\$ I had completely missed the wildcard import. Nice catch :) \$\endgroup\$ Commented Jan 10, 2018 at 13:05
  • \$\begingroup\$ I actually don't mind wildcard imports in tkinter -- it's idiomatic there (even if it's idiotic elsewhere). \$\endgroup\$ Commented Jan 10, 2018 at 18:24
  • 2
    \$\begingroup\$ @AdamSmith: I wouldn't say it's idiomatic. Pervasive, perhaps, probably because one guy chose to do it when writing one of the first tutorials and everyone copied from that one source. It's bad for Tkinter for the same reasons it's bad for anything else - it pollutes the global namespace and makes the code a bit harder to understand. However, at the end of the day it's mostly personal preference. \$\endgroup\$ Commented Jan 10, 2018 at 18:55
  • \$\begingroup\$ @BryanOakley ehh that's splitting hairs though isn't it? Idiomatic means little apart from "pervasive [in its context]," and though I agree that explicit namespacing is better in the general case, "A Foolish Consistency" wins through for me. If you're writing a tkinter GUI, write it like tkinter GUIs are written, which (in my subjective experience) includes from tkinter import *. That said: I'm aware I'm bikeshedding a bit ;) \$\endgroup\$ Commented Jan 10, 2018 at 20:52
5
\$\begingroup\$

In addition to the two previous answers, you should keep the initializer (__init__()) light and minimalist, and delegate most of the job to different functions each fulfilling one simple goal (You may throw a glance to Tkinter best practices). Also I find the individual functions to be quite long and an improved design would lead to getting rid of the global variables in use

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

Using files to store large amounts of data can be problematic causing data ambiguities, data islands, etc. You'd be better off using a well designed database, which you then manipulate with SQL. Check out the dba section of stack overflow for more info. You also might want to write your user-interface in HTML, so a company could easily use it without installing python and tk.

\$\endgroup\$
3
  • \$\begingroup\$ i plan on converting this into a exe before selling it or allowing a company to use it. There will be no need for a windows based computer to download any thing programming related. \$\endgroup\$ Commented Jan 11, 2018 at 12:20
  • \$\begingroup\$ I'm a little bias toward HTML's client-server architecture. Using it isn't necessary, but without it you will have to manage some things you otherwise wouldn't have to. A few things that come to mind are: version control and installation across multiple machines, security (which machines can you install it on vs https with a password), and making calls to a remote DB server via FTP or something. Of course, using an exe has advantages too. Ultimately you have to make these decisions yourself. \$\endgroup\$ Commented Jan 11, 2018 at 17:13
  • \$\begingroup\$ No , that is a great point , i didnt take in the possibility of multi platforms installing the application \$\endgroup\$ Commented Jan 12, 2018 at 12:35

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.