Setting instance variable for salary bonus based on academic position

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:

self.name = name
self.salary = salary

self.bonus = 70000
self.bonus = 50000
self.bonus = 30000

• It's likely that it doesn't even make sense to set such an instance variable in the constructor. But we can't really advise you properly based on the tiny code excerpt you've shown here. Nov 20, 2021 at 17:28

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

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

self.name = name
self.salary = salary

@property
def bonus(self):
return bonus

raise ValueError(
)


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

class Scholar:
# ...

@property
def bonus(self):


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:
'Professor': 70000,
'Associate Professor': 50000,
'Assistance Professor': 30000,
}

self.name = name
self.salary = salary

self.bonus = self.get_bonus()

def get_bonus(self):
return bonus

raise ValueError(
)

• @ Grajdeano Alex thanks you so much for your answers. so basically @property is like an attribute which has immediate invoked call? we can access simply just like it is an instance variable?
– tmo
Nov 19, 2021 at 13:53
• and you're suggesting that I should write get_bonus() instead of the @property bonus in order to protect the variable from changing outside the class?
– tmo
Nov 19, 2021 at 13:56
• Basically yes. For a deeper read have at this Nov 19, 2021 at 14:10
• Iterating over a dict to check a value for equality against each key seems to defeat the purpose of using a dict.
– FMc
Nov 19, 2021 at 18:18
• @FMc yep, that's correct. It was pretty late when I wrote this. Added extra-comments for that part now. Nov 20, 2021 at 10:16

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?
}

self.name = name
self.salary = salary

# 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):

# 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

if value in self.BONUSES:
else:
# Adjust the error message as needed to explain the problem
# in more detail or to provide allowed values.


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,
}

# Here you get minimal error handling for free, as a KeyError.
# If desired, wrap in a try-except structure to raise

• Thanks. For your question if it is mutable or not, well, I just started learning OOP in python. the exercise just tells me to create a class Scholar and compute the yearly income which include bonus. so the answer would be.. not concerned!
– tmo
Nov 20, 2021 at 3:25