S.O.L.I.D Essentials missing points?

Question

I've read so many articles about this but still I have 2 questions.

Question #1 - Regarding Dependency Inversion :

It states that high-level classes should not depend on low-level classes. Both should depend on abstractions. Abstractions should not depend on details. Details should depend on abstractions.

for example :

public class BirthdayCalculator
{
    private readonly List<Birthday> _birthdays;

    public BirthdayCalculator()
    {
        _birthdays = new List<Birthday>();// <----- here is a dependency
    }
...

The fix : put it in a ctor.

public class BirthdayCalculator
{
    private readonly IList<Birthday> _birthdays;

    public BirthdayCalculator(IList<Birthday> birthdays) 
    {
        _birthdays = birthdays;
    }

• If it will be in ctor - I'll have to send it each and every time I use the class. So I will have to keep it when calling the BirthdayCalculator class. it it ok to do it like that ?

• I can argue that , after the fix , still - IList<Birthday> _birthdays should not by there ( the Birthday in IList) - but it should be as IList<IBirthday>. Am I right ?

Question #2 - Regarding Liskov Substitution :

derived classes must be substitutable for their base classes

or more accurate :

Let q(x) be a property provable about objects x of type T. Then q(y) should be true for objects y of type S where S is a subtype of T.

(already read this )

example :

public abstract class Account
{
     public abstract void Deposit(double amount);
}

I have a class :

public class CheckingAccount : Account
{
     public override void Deposit(double amount)
    {    ...
        _currentBalance += amount;
    }
}

and the bank wants to open a Mortgage account - so :

public class MortgageAccount : Account
{

    public override void Deposit(double amount)
    {
        _currentBalance -= amount; //<-----notice the minus
    }
}

The problem arises when there is a function which accepts mortage as Account and do deposit.

public class Bank
{
    public void ReceiveMoney(Account account, double amount)
    {
        double oldBalance = account.CurrentBalance;
        account.Deposit(amount); //oopssss?????
    }
}

so here , it violates the LSP.

But I don't understand.

every overridden method will do different code when overridden so it will never be 100% replaceable !

the definition did NOT talk about " logic should continue as in base class ( always deposit(add) positive numbers)"

example:

What if both CheckingAccount class and MortgageAccount class were depositing positive numbers but MortgageAccount also log to db ? does it still breaks LSP ? What is the boundry for breaks/not-brakes LSP ?

the definition should define whatis that boundry. and it isn't saying anything about it.

what am i missing ?

Answer 1

LSP says that any promises the base class makes, subclasses must also make. In the Account case, yes, that means Deposit subtracting from a Mortgage's balance is rather messed up. One workaround for this would be if you consider the balance to be the difference between the amount the customer owes you and how much you owe him. (I'm about 54% sure that's the original meaning of "balance" anyway.) A positive balance might mean the customer has money, while a negative one might mean he owes money. If you can't resolve it so that you can treat the two accounts similarly, then they should not be related -- or at least, the "Deposit" method should not be defined on the base class.

As far as DIP, what it doesn't really mention is that it's not meant to apply to every line of code. Eventually you have to have a concrete class somewhere. The point is to limit reliance on the specifics of those concrete classes to sections of code that absolutely have to know about them, and keeping the number and size of those sections as small as possible. That means using as generic an interface as you can get away with, as much as you can. For example, if you don't need all the guarantees that List makes (order of enumeration, presence of duplicates, nulls, etc), then you might declare _birthdays as an IEnumerable instead. If the constructor is the only thing that knows that your IEnumerable is actually a List, then you're more-or-less adhering to the principle.

(Be warned, though: this doesn't mean you can adhere to DIP by simply declaring everything as an Object and downcasting as needed. Downcasting itself could be considered a violation of DIP, as you're no longer relying on the interface you've been provided; you're trying to get a more specific one.)

Answer 2

If it will be in ctor - I'll have to send it each and every time I use the class. So I will have to keep it when calling the BirthdayCalculator class. it it ok to do it like that?

I would retrieve the list of birthday's once, make one BirthdayCalculator, and pass that around. You should only need to instantiate a new BirthdayCalculator if your list of birthdays change, or if it maintains state somehow.

I can argue that , after the fix , still - IList<Birthday> _birthdays should not by there ( the Birthday in IList) - but it should be as IList<IBirthday>. Am I right ?

This depends. Is Birthday just a data object? Is there any logic contained within it? If it is just a bunch of properties / fields, then an interface is overkill.

If Birthday does have logic, then I would go with IEnumerable<IBirthday>. List<Birthday> is not assignable to IList<IBirthday>, so you could have issues there. If you don't need to manipulate it, stick with IEnumerable.

---

Regarding your second question, the only thing I have a problem with is your naming convention. You don't make deposits to a Mortage. I think if you were to rename that method it might make more sense.

Also, a checking account and a mortage account are two very different animals. One is a deposit account, where as the other is a loan. I would be very careful with trying to use them interchangeably.

Edit

Others have noted that you could make _currentBalance in MortageAccount a negative value, then add to it. If that is easier for you to understand, then go for it. However, the internals of how a MortageAccount handles its balance is not important, so long as GetBalance or whatever method you use the check / display it, handles it properly.