0

The following class breaks the open/closed principal:

public class Account
{
    public decimal Interest { get; set; }
    public decimal Balance { get; set; }

    public decimal CalcInterest(string accType)
    {

        if (accType == "Regular")
        {
            Interest = (Balance * 4) / 100;
            if (Balance < 1000) Interest -= (Balance * 2) / 100;
            if (Balance < 50000) Interest += (Balance * 4) / 100;
        }
        else if (accType == "Salary") // salary savings
        {
            Interest = (Balance * 5) / 100;
        }
        else if (accType == "Corporate") // Corporate
        {
            Interest = (Balance * 3) / 100;
        }
        return Interest;
    }
}

And it can be fixed like this:

public interface IAccount
{
    decimal Balance { get; set; }
    decimal CalcInterest();
}

public class RegularAccount : IAccount
{
    public decimal Balance { get; set; }

    public decimal CalcInterest()
    {
        var interest = (Balance * 4) / 100;

        if (Balance < 1000)
            interest -= (Balance * 2) / 100;

        if (Balance < 50000)
            interest += (Balance * 4) / 100;

        return interest;
    }
}

public class SalaryAccount : IAccount
{
    public decimal Balance { get; set; }

    public decimal CalcInterest()
    {
        return (Balance * 5) / 100;
    }
}

public class CorporateAccount : IAccount
{
    public decimal Balance { get; set; }

    public decimal CalcInterest()
    {
        return (Balance * 3) / 100;
    }
}

However the RegularAccount still breaks the principal due to the balance checks. How do you fix this?

Is the answer just to create more classes that implement the IAccount interface? e.g. Regular1000Account:IAccount, Regular5000Account:IAccount, Regular8000Account:IAccount, etc

Sun
  • 4,458
  • 14
  • 66
  • 108
  • 5
    "*However the RegularAccount still breaks the principal due to the balance checks*" -- what makes you think this? The original violates open/closed because if there are *multiple* places which switch on the account type, and a 4th account type was added, all of the places which switched on the account type would have to be updated. That probably isn't the case for the balance checks in the regular account. A good rule of thumb for the open/closed principle is "If I change this, how many other bits of code do I need to find and fix" – canton7 Jan 09 '20 at 16:36
  • 1
    See my other answer here: https://stackoverflow.com/questions/58472382/what-do-i-need-to-change-to-implement-solid-design-correctly/58472600#58472600 – canton7 Jan 09 '20 at 16:38
  • Ah ok. I've been sorta wondering if this was OK myself. Thanks for your input. – Sun Jan 09 '20 at 16:40
  • Perfect, just read your other answer – Sun Jan 09 '20 at 16:41
  • As I often point out on these sorts of beginner exercises: a lot of these problems go away when you realize that real world bank account software is absolutely nothing like this. A real-world bank account representation is *an append-only list of all deposits and withdrawals*, not just a balance. – Eric Lippert Jan 09 '20 at 19:03
  • 2
    Another way to solve your problem is to make bank accounts not responsible for computing interest, because that is not a concern of the account. That's a concern of an *interest policy*, so make a separate class for interest policies, and the account then is *composed of a policy and the other account data*. – Eric Lippert Jan 09 '20 at 19:04
  • Thanks @EricLippert – Sun Jan 09 '20 at 19:12

0 Answers0