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\$
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\$
  • \$\begingroup\$ woah , you got some great points ,so you think sqlite3 would be better? , i use sublime text 3 as a enviorment btw \$\endgroup\$ – Ron Computing Jan 10 '18 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\$ – Ron Computing Jan 10 '18 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\$ – Mast Jan 10 '18 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\$ – hjpotter92 Jan 10 '18 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\$ – Captain Man Jan 10 '18 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\$
  • \$\begingroup\$ great point i will look into organizing my code in groups seeing as though that will be eaiser to manage \$\endgroup\$ – Ron Computing Jan 10 '18 at 12:22
  • \$\begingroup\$ I had completely missed the wildcard import. Nice catch :) \$\endgroup\$ – hjpotter92 Jan 10 '18 at 13:05
  • \$\begingroup\$ I actually don't mind wildcard imports in tkinter -- it's idiomatic there (even if it's idiotic elsewhere). \$\endgroup\$ – Adam Smith Jan 10 '18 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\$ – Bryan Oakley Jan 10 '18 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\$ – Adam Smith Jan 10 '18 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\$
  • \$\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\$ – Ron Computing Jan 11 '18 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\$ – user4757074 Jan 11 '18 at 17:13
  • \$\begingroup\$ No , that is a great point , i didnt take in the possibility of multi platforms installing the application \$\endgroup\$ – Ron Computing Jan 12 '18 at 12:35

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.