Setting instance variable for salary bonus based on academic position

Question

I have a class Scholar which is basically a teacher in a university. I have to calculate the yearly salary of a scholar instance where the formula is 12-month salary + bonus
the bonus is determined by the academic_position
if the teacher is:

• a Professor --> bonus = 70000
• an Associate Professor --> bonus = 50000
• an Assistant Professor --> bonus = 30000

I want to keep the bonus in a separate variable so i implemented it in __init__, but it looks messy. not sure if it is the way to do or not. do you have any suggestion on my code? Is there a way to improve readibility? what normally a pythonista do with this kind of problem?

class Scholar:
  def __init__(self, name, salary, academic_position):

    self.name = name
    self.salary = salary
    self.academic_position = academic_position

    if self.academic_position == 'Professor':
      self.bonus = 70000
    elif self.academic_position == 'Associate Professor':
      self.bonus = 50000
    elif self.academic_position == 'Assistance Professor':
      self.bonus = 30000

Answer 1

If you have to stick to using this specific structure, I'd do it like this:

class Scholar:
    _ACADEMIC_POSITIONS_BONUS_MAP = {
        'Professor': 70000,
        'Associate Professor': 50000,
        'Assistance Professor': 30000,
    }

    def __init__(self, name, salary, academic_position):
        self.name = name
        self.salary = salary
        self.academic_position = academic_position

    @property
    def bonus(self):
        for academic_position, bonus in self._ACADEMIC_POSITIONS_BONUS_MAP.items():
            if self.academic_position == academic_position:
                return bonus

        raise ValueError(
            f"Invalid academic position. Allowed: "
            f"{', '.join(self._ACADEMIC_POSITIONS_BONUS_MAP)}"
        )

And as other have mentioned, instead of iterating through the dict itself you can just do this:

class Scholar:
    # ...

    @property
    def bonus(self):
        return self._ACADEMIC_POSITIONS_BONUS_MAP[self.academic_position]

We've created a map between each type of academic_position and its specific bonus and a @property where we return the bonus.

And then you can use it like this:

professor = Scholar("John Doe", 30000, "Professor")
associate_professor = Scholar("Associate John Doe", 30000, "Associate Professor")
assistance_professor = Scholar("Assistance John Doe", 30000, "Assistance Professor")


print(professor.bonus)
print(associate_professor.bonus)
print(assistance_professor.bonus)

Now, when you need to add another academic_position, you just have to modify _ACADEMIC_POSITIONS_BONUS_MAP.

Next, in order to calculate the yearly salary, just create another method within your class:

class Scholar:
    # ...

    def get_yearly_salary(self):
        # Note: multiply the salary with 12 if it's monthly-based
        return self.salary + self.bonus

Note: I'm not sure if @property is the best way of doing this. You could've just done something along the lines:

class Scholar:
    _ACADEMIC_POSITIONS_BONUS_MAP = {
        'Professor': 70000,
        'Associate Professor': 50000,
        'Assistance Professor': 30000,
    }

    def __init__(self, name, salary, academic_position):
        self.name = name
        self.salary = salary
        self.academic_position = academic_position

        self.bonus = self.get_bonus()

    def get_bonus(self):
        for academic_position, bonus in self._ACADEMIC_POSITIONS_BONUS_MAP.items():
            if self.academic_position == academic_position:
                return bonus

        raise ValueError(
            f"Invalid academic position. Allowed: "
            f"{', '.join(self._ACADEMIC_POSITIONS_BONUS_MAP)}"
        )

Answer 2

You have a small class where the position must be one of three allowed values and where the bonus is derived purely from the position. Under typical conditions, this implies that you should validate the position and compute the bonus dynamically based on position (don't store the bonus, compute it). The way to achieve both of those things in via properties: a getter for bonus and both a getter and setter for position. Here's an illustration with some additional explanations in the comments.

class Scholar:

    # A data structure that links positions to bonuses and also
    # provides a mechanism to validate the position itself.

    BONUSES = {
        'Professor': 70000,
        'Associate Professor': 50000,
        'Assistance Professor': 30000,   # Shouldn't this be Assistant?
    }

    def __init__(self, name, salary, academic_position):
        self.name = name
        self.salary = salary
        self.academic_position = academic_position

    # Bonus is always computed dynamically, so position and bonus
    # are never out of sync. Since we do not want users of the Scholar
    # class to set the bonus, we only define the getter property.

    @property
    def bonus(self):
        return self.BONUSES[self.academic_position]

    # Position requires actual validation, so we need both a getter and a
    # setter. The setter raises an error if the position is invalid, and the
    # actual attribute is stored under the same name with an underscore prefix,
    # which conveys the idea to class users that the attribute is intended to
    # be managed by the class, not by users.

    @property
    def academic_position(self):
        return self._academic_position

    @academic_position.setter
    def academic_position(self, value):
        if value in self.BONUSES:
            self._academic_position = value
        else:
            # Adjust the error message as needed to explain the problem
            # in more detail or to provide allowed values.
            raise ValueError('Invalid academic_position')

That's the garden-variety way to handle stuff like this, in the sense that you'll find many online tutorials covering approaches like it. One thing to notice is that properties are a bit of a pain: they tend to spawn a lot of boilerplate code, and the whole things starts to feel too much like an unpleasant language that shall not be named.

Before going to this trouble I would first ask a deeper question: should Scholar instance be mutable? Based on what you've told us so far, the answer could be no. In that case, Scholar could be a namedtuple or dataclass. For example:

from collections import namedtuple

Scholar = namedtuple('Scholar', 'name salary academic_position bonus')

BONUSES = {
    'Professor': 70000,
    'Associate Professor': 50000,
    'Assistant Professor': 30000,
}

def make_scholar(name, salary, academic_position):
    # Here you get minimal error handling for free, as a KeyError.
    # If desired, wrap in a try-except structure to raise
    # a more informative ValueError.
    bonus = BONUSES[academic_position]
    return Scholar(name, salary, academic_position, bonus)