Printing employee object information from a dictionary of employees

Question

I am attempting to print employee object information from a dictionary of employee objects without using a for loop. Here is what I have done so far:

employee_dict = {}

class Employee:

  def __init__(self, id, salary):
    self.id = id
    self.salary = salary
    self.employee_dictionary(self)

  def info(self):
    return "Employee ID:{} \nSalary:{}".format(self.id, self.salary)

  def employee_dictionary(self):
    employee_dict = {self.id: self}

emp = Employee(1, 10)
emp1 = Employee(2, 5)

employee = employee_dict[1]

print(employee.info())

Could someone point me in the right direction? I feel like I am close. The error this code gives me is:

Traceback (most recent call last):
  File "/home/rob/PycharmProjects/untitled4/sdfghsfdgjrtyrty.py", line 19, in <module>
    employee = employee_dict[1]
KeyError: 1

Answer 1

The underlying problem here is that you're not using the dictionary right. Instead of adding the new employee to the existing global employee_dict, you're trying to replace that global dict with a new one, containing just the new employee:

def employee_dictionary(self):
  employee_dict = {self.id: self}

If you fix this, the problem you're actually asking about won't even come up:

def employee_dictionary(self):
  employee_dict[self.id] = self

This will add a new mapping from self.id to self in the dict. So, after your two Employee constructions, with IDs 1 and 2, you'll end up with two entries in the dict, for keys 1 and 2, and everything will just work.

---

Also, you should definitely at least consider two other changes:

• Replace the global variable with a class attribute, and possibly also replace the employee_dictionary method with a class method, as detailed in scharette's answer.
• Rename the method from employee_dictionary to something (a) marked private (methods starting with _ are private by convention, meaning your users know they aren't supposed to be calling it unless they have a weird use case), and (b) more reflective of what it does. Maybe _register or _add_to_dict?

The first one of these would also (as, again, detailed in scharette's answer) have made the problem you're asking about go away. (But you still need the main fix anyway.)

---

But you probably want to understand the more immediate scope problem anyway—and how to solve it when you can't just make it go away.

In Python, any name that you assign to in a function is a local variable. If there's a spam = … anywhere in the function (or a with eggs as spam: or certainly other kinds of things that also count as assignment), every reference to spam in that function is to that local. So, employee_dict = {self.id: self} creates a local variable named employee_dict, assigns a value to it, and then returns, at which point all the locals go away. The fact that it happens to have the same name as a global doesn't mean anything to Python (although to a human reader, like you, it's obviously confusing).

Any name that you use without assigning to it anywhere is a local-or-enclosing-or-global-or-builtin variable. When you do employee_dict[self.id], because there's no employee_dict = … anywhere, Python searches the local, enclosing, global, and builtin scopes to find what you meant by employee_dict, and it finds the global.

You can force Python to treat a name as a global variable, even if you assign to it, with a global statement at the top of the function:

def employee_dictionary(self):
  global employee_dict
  employee_dict = {self.id: self}

Adding global is safe even if the name would already be a global. This is usually pointless—but if you're not sure whether something counts as an assignment (or just not sure future readers of your code would be sure…), and you want to make it clear that the variable is a global, you can declare it:

def employee_dictionary(self):
  global employee_dict
  employee_dict[self.id] = self

Answer 2

This is a scope problem. employee_dict = {} is not acting like you think.

You're not assigning the value in the right scope. Therefore, you're never actually adding an Employee to it. Which cause the KeyError.

What you are looking for in your case is probably using a class variable like so,

class Employee:
    employee_dict = {}
    def __init__(self, id, salary):
        self.id = id
        self.salary = salary
        self.employee_dictonary(self)

    def info(self):
        return "Employee ID:{} \nSalary:{}".format(self.id, self.salary)

    @classmethod
    def employee_dictonary(cls,current_employee):
        cls.employee_dict[current_employee.id]=current_employee

emp = Employee(1, 10)
emp1 = Employee(2, 5)

print(Employee.employee_dict[1].info())

Basically, for the sake of explanation, you're creating a employee_dict which will be share among all instances of Employee.

Also, there's one last thing you have to keep in mind is if someone create 2 employee like so,

emp = Employee(1, 10)
emp2 = Employee(1, 100)

calling print(Employee.employee_dict[1].info()) will output

Employee ID:1 
Salary:100

which is probably what you want, but still thought it was relevant to underline that an employee can be overridden.

Answer 3

You should define employee_dict as global:

def employee_dictonary(self):
  global employee_dict

Without the global keyword you are assigning to local variable.

The second problem is using employee_dict = {self.id: self} for assigning to employee_dict. With that, you overwriting the variable with each call. That line should be employee_dict[self.id] = self.

With that change, the output is:

Employee ID:1 
Salary:10

Using global variables is strongly discouraged in most cases, see Why are global variables evil?.