Insurance premium calculator exercise

Question

I attempted a coding exercise during an interview, defined & implemented here (readme for problem):

https://gitlab.com/vineet_rc/health-insurance-premium-quote-generator-2

It was initially a simple and single java class program.

public double calculatePremium(Person person) {
    // Added base premium
    double premium = 5000;

    // Increase premium depending on age
    if (person.getAge() >= 18) {
        premium *= 1.1;
    }
    if (person.getAge() >= 25) {
        premium *= 1.1;
    }
    if (person.getAge() >= 30) {
        premium *= 1.1;
    }
    if (person.getAge() >= 35) {
        premium *= 1.1;
    }
    if (person.getAge() >= 40) {
        // Add 20% per 5 years above 40
        int age = person.getAge() - 40;
        while (age >= 5) {
            premium *= 1.2;
            age -= 5;
        }
    }

    // Add gender specific adjustments
    if (person.getGender() == Gender.MALE) {
        premium *= 1.02;
    }

    // Increase 1% per health issue
    for (HealthIssues issue : person.getHealthIssues()) {
        premium *= 1.01;
    }

    Set<Habbits> habbitsList = person.getHabbits();
    // Decrease 3% per good habbit
    for (Habbits habbit : habbitsList) {
        if (Constants.goodHabbits.contains(habbit)) {
            premium *= 0.97;
        }
    }
    // Increase 3% per good habbit
    for (Habbits habbit : habbitsList) {
        if (Constants.badHabbits.contains(habbit)) {
            premium *= 1.03;
        }
    }

    return premium;
}

I only had three JUnits with different Personobjects initially. The reviewer mentioned if the JUnit failed, we wouldn't know exactly where it failed.

So I took cue & cut the various calculations & gave it its own method. Then called them in my original calculatePremium() method.

public double calculatePremium(Person person) {
    // Added base premium
    double premium = 5000;
    premium = this.applyAgeBasedCalcs(person, premium);
    premium = this.applyGenderBasedCalcs(person, premium);
    premium = this.applyHealthIssuesBasedCalcs(person, premium);
    premium = this.applyGoodHabbitsBasedCalcs(person, premium);
    premium = this.applyBadHabbitsBasedCalcs(person, premium);
    return premium;
}

And of course added JUnits for them (not an exhaustive set). So instead of testing only for the final answer, we're also testing if the individual calculating components are working fine or not.

Finally the reviewer mentioned that if he ever wanted to change the order or the number of apply* methods called (add new ones), but without changing this class every time, how would one do about it? He still may want to use this original version of the rules. I immediately explained the strategy pattern & upon request, implemented it.

I merely made an interface (PremiumCalculationStrategy) & a single class that implements it (NormalPremiumCalculationStrategy) for now.

public double calculatePremium(Person person) {
    // Added base premium
    double premium = 5000;
    premium = Rules.applyAgeBasedCalcs(person, premium);
    premium = Rules.applyGenderBasedCalcs(person, premium);
    premium = Rules.applyHealthIssuesBasedCalcs(person, premium);
    premium = Rules.applyGoodHabbitsBasedCalcs(person, premium);
    premium = Rules.applyBadHabbitsBasedCalcs(person, premium);
    return premium;
}

I made all the apply* methods static as I didn't see the need to instantiate a Rules object; there's no state & the method is functional.

Unfortunately, that wasn't well appreciated. He didn't like the static methods & said I broke all the OO principles. He suggested that I should make it more object-oriented. Additionally just because there's a class & interface, doesn't mean it's a strategy pattern.

I didn't understand the comments too well. Could someone review this & let me know of a good way to code this, or just why is my implementation so bad? What's a good way to make it more object-oriented, and what's the right usage of a strategy pattern?

Answer 1

A good OOP design is open for extension and closed for modification. The single-method implementation is very far from that.

I think the purpose of the exercise is to be able to build a premium calculator from parts, in a way that you can easily modify the parts that are likely to change. What are the parts?

• The base premium
• Calculate percentage modifier based on conditions
  • There is a list of conditions, with associated percentage increase or decrease

A percentage modifier part is essentially a function that takes a Person and returns a percentage. You could provide such parts to make it easy to build a calculator, for example:

interface PremiumPercentCalculator extends Function<Person, Integer> {}

PremiumPercentCalculator male(int percent) {
    return person -> person.male ? percent : 0;
}

PremiumPercentCalculator preConditions(int percent) {
    return person -> person.preConditions().size() * percent;
}

PremiumPercentCalculator goodHabits(int percent) {
    return person -> person.habits().stream().filter(Habit::isGood).mapToInt(h -> percent).sum();
}

PremiumPercentCalculator badHabits(int percent) {
    return person -> person.habits().stream().filter(h -> !h.isGood()).mapToInt(h -> percent).sum();
}

Then, using the builder pattern, you could build a calculator from parts like this:

float premium = new Calculator(person, 5000)
    .add(p -> getPremiumPercentByAge(p))
    .add(male(2))
    .add(preConditions(1))
    .add(badHabits(3))
    .add(goodHabits(-3))
    .calculate();

Imagine that there is a UI for an insurance agent, where he can drag and drop widgets that correspond to condition types like "is male", "pre-conditions", and so on, each with an option to modify the percentage to apply. Such a UI could be easily built from the parts we provided, and new parts can be easily added by adding more PremiumPercentCalculator implementations.

Notice that person and the base premium 5000 are constructor parameters, as they are absolutely essential for the calculator, so it made sense to make them required up front, so that the rest of the implementation can count on them to exist without further validation.

You might notice that badHabits and goodHabits iterate over the collection of habits twice. This might very well be negligible. But if not, it's easy enough to fix, by adding another part:

PremiumPercentCalculator habits(int goodPercent, int badPercent) {
    return person -> person.habits().stream().mapToInt(h -> h.isGood() ? goodPercent : badPercent).sum();
}

Admittedly getPremiumPercentByAge is not so easy to make extensible, this part can certainly be improved, but I think that was not the most important part of the interview.

Note that although I used lambdas for compact expressions, the concept doesn't require it, and although the solution will be more verbose with classes fully written out, that won't diminish its benefits.