2
\$\begingroup\$

I created a python class called Score, I intend to use it to assign scores depending on 2 parameters, first the type of score - score name and second the user_input, so for example if I want to get the score of loan term, I will enter 'loan_term' as the first parameter and loan term value 36 as the second parameter; a function in the class then calculates the score and returns the value.

The idea is to be able to re-use this class as and when needed and to be able to modify the code outside of the main application code.

So I will like someone to review this class implementation and let me know if I have deviated from known standards or is there a better way to implement this?

class Score:
    def __init__(self, score_name):
        self.score_name = score_name

    def get_score_name(self):
        return self.score_name

    def get_score(get_score_name, user_input):
        # loan term score grading
        if get_score_name == 'loan_term' and user_input <= 18:
            i = 5
        elif get_score_name == 'loan_term' and user_input <= 24:
            i = 4
        elif get_score_name == 'loan_term' and user_input <= 48:
            i = 3
        # loan reason score grading
        elif get_score_name == 'loan_reason' and user_input == 0:
            i = 3.5
        elif get_score_name == 'loan_reason' and user_input == 1:
            i = 2.0
        elif get_score_name == 'loan_reason' and user_input == 2:
            i = 1.5
        elif get_score_name == 'loan_reason' and user_input == 3:
            i = 2.5
        elif get_score_name == 'loan_reason' and user_input == 4:
            i = 1.5
        elif get_score_name == 'loan_reason' and user_input == 5:
            i = 2.0
        elif get_score_name == 'loan_reason' and user_input == 6:
            i = 0.5
        # job security score grading
        elif get_score_name == 'job_security' and user_input == 0:
            i = 1.0
        elif get_score_name == 'job_security' and user_input == 1:
            i = 2.0
        elif get_score_name == 'job_security' and user_input == 2:
            i = 3.0
        elif get_score_name == 'job_security' and user_input == 3:
            i = 4.0
        elif get_score_name == 'job_security' and user_input == 4:
            i = 5.0
        # average apr score grading
        elif get_score_name == 'average_apr' and user_input <= -1.0:
            i = 5
        elif get_score_name == 'average_apr' and user_input <= -0.6:
            i = 4
        elif get_score_name == 'average_apr' and user_input < 0.5:
            i = 3
        elif get_score_name == 'average_apr' and user_input <= 1.0:
            i = 2
        elif get_score_name == 'average_apr' and user_input > 1.0:
            i = 1
        # monthly expenses score grading
        elif get_score_name == 'monthly_expenses' and user_input < 0.10:
            i = 5
        elif get_score_name == 'monthly_expenses' and user_input < 0.20:
            i = 4
        elif get_score_name == 'monthly_expenses' and user_input < 0.30:
            i = 3
        elif get_score_name == 'monthly_expenses' and user_input < 0.40:
            i = 1
        elif get_score_name == 'monthly_expenses' and user_input > 0.40:
            i = 0
        # monthly surplus score grading
        elif get_score_name == 'monthly_surplus' and user_input < 0.20:
            i = 5
        elif get_score_name == 'monthly_surplus' and user_input < 0.35:
            i = 4
        elif get_score_name == 'monthly_surplus' and user_input < 0.45:
            i = 3
        elif get_score_name == 'monthly_surplus' and user_input <= 0.60:
            i = 2
        elif get_score_name == 'monthly_surplus' and user_input > 0.60:
            i = 1
        else:
            i = 0

        return i

# # loan term test

# loan_term_score = Score.get_score(Score('loan_term').get_score_name(), 21)
# print(f'loan term  score: {loan_term_score}')


# # loan reason test

# loan_reason_score = Score.get_score(Score('loan_reason').get_score_name(), 2)
# print(f'loan reason score: {loan_reason_score}')

# # job security test

# job_security_score = Score.get_score(Score('job_security').get_score_name(), 3)
# print(f'job security score: {job_security_score}')


# # average apr test

# average_apr_score = Score.get_score(Score('average_apr').get_score_name(), 1.0)
# print(f'average apr score: {average_apr_score}')

# # monthly expenses test

# monthly_expenses_score = Score.get_score(Score('monthly_expenses').get_score_name(), 0.15)
# print(f'monthly expenses score: {monthly_expenses_score}')

# # monthly surplus test

# monthly_surplus_score = Score.get_score(Score('monthly_surplus').get_score_name(), 0.55)
# print(f'monthly surplus score: {monthly_surplus_score}')
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to code review! I hope you get some good answers. \$\endgroup\$
    – pacmaninbw
    Jul 13, 2020 at 11:30

1 Answer 1

1
\$\begingroup\$

Bug

    elif get_score_name == 'monthly_expenses' and user_input < 0.40:
        i = 1
    elif get_score_name == 'monthly_expenses' and user_input > 0.40:
        i = 0

What happens if user_input is exactly 4.0?

General algorithm

Those sequential ifs should go away. Maintain a dictionary where the key is a score name and the value is a sorted tuple of 2-tuples: input threshold and score. Look up the outer tuple by score name, then use bisect to do efficient lookup of the applicable score.

Score name

get_score_name as a method name is fine. get_score_name as a parameter name is confusing and should simply be score_name, though that parameter should not even exist since you already have self.score_name.

Example code

Note:

  • I've taken liberties with some of your boundary conditions that didn't make sense; in particular the upper boundary for monthly_surplus
  • This approach allows for some slightly silly things to happen, like loan_reason and job_security coercing floating-point answers to integers, when - for these two categories - floating-point doesn't make sense
  • user_input is put in a tuple for easy comparison to the other threshold tuples
from bisect import bisect
from typing import Tuple, Dict, Optional

ScoreName = str
Score = float
Threshold = float
ScorePair = Tuple[Threshold, Score]
ScoreDict = Dict[ScoreName, Tuple[ScorePair, ...]]

INF = float('inf')

THRESHOLDS: ScoreDict = {
    'loan_term': (
        (18, 5),
        (24, 4),
        (48, 3),
    ),
    'loan_reason': (
        (0, 3.5),
        (1, 2.0),
        (2, 1.5),
        (3, 2.5),
        (4, 1.5),
        (5, 2.0),
        (6, 0.5),
    ),
    'job_security': (
        (0, 1),
        (1, 2),
        (2, 3),
        (3, 4),
        (4, 5),
    ),
    'average_apr': (
        (-1.0, 5),
        (-0.6, 4),
        ( 0.5, 3),
        ( 1.0, 2),
        ( INF, 1),
    ),
    'monthly_expenses': (
        (0.1, 5),
        (0.2, 4),
        (0.3, 3),
        (0.4, 1),
        (INF, 0),
    ),
    'monthly_surplus': (
        (0.20, 5),
        (0.35, 4),
        (0.45, 3),
        (0.60, 2),
        ( INF, 1),
    ),
}


def get_score(score_name: ScoreName, user_input: Threshold) -> Optional[Score]:
    thresholds = THRESHOLDS[score_name]
    index = bisect(thresholds, (user_input,))
    if index >= len(thresholds):
        return None
    return thresholds[index][1]


def test():
    assert get_score('loan_term', 17) == 5
    assert get_score('loan_term', 18) == 5
    assert get_score('loan_term', 48) == 3
    assert get_score('loan_term', 49) is None
    assert get_score('monthly_surplus', 0.62) == 1


if __name__ == '__main__':
    test()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.