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.

Answer 3

#1

• Yes, you will have to call that every time you create the object. This is where Factories and Inversion of Control frameworks comes in handy, to remove all the plumbing you'd have to do.

• Yes, you should be using an IBirthday interface.

#2

Your logic is flawed here. When you deposit money into an account you're adding money to it. The mortgage account doesn't have money in the account (a positive balance), and you're not removing money from it by depositing. The mortgage account has a negative balance, and depositing into the account lowers the balance (adding to a negative balance).

Answer 4

Regarding LSP: As I see it, LSP is mostly about (public) contract of a hierarchy. You have an account (object), you can credit/debit money to it (contract). For mortgage account Credit will result in subtraction and Debit will result in addition. For checking account it's the other way around.

The important thing is that given an account you can always ask it to do credit/debit actions and that all subclasses of an account object will support those operations.

Answer 5

Dependency Inversion:

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 ?

You need it not when using the object but when constructing the BirthdayCalendar object. Moving the IList<Birthday> to the construction interface ensures that all the dependencies of BirthdayCalendar are same as the dependencies of the caller (i.e., no new ones), and that's what we're trying to achieve.

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 ?

That's not related directly to the dependency inversion principle. You have offloaded the dependency to the caller and now you're asking about the details of this dependency. It might be a good idea to use IBirthday to access the internals of you list instead of Birthday, but that a question of your interfaces, unrelated to the dependency inversion.

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.

Liskov Substitution Principle is applicable to base class vs. derived class relation. Your example deals with 2 classes inheriting from a shared base, so your relation is derived class1 vs. derived class2. This is the exact opposite of what LSP is talking about, exactly for the reasons you mentioned - the logic is different in both derived classes. LSP is a measure for validity of the inheritance that you defined. If you can answer positively to the question is my derived class object can be used transparently everywhere my base class object is used?, you're not breaking the LSP. Otherwise, you are breaking it.

Answer 6

With regard to the first point, it's fine to use a specific low-level class as a private implementation detail of the other class; the code might be more SOLID if one were to say IList<Birthday> _birthdays = new List<Birthday>; but performance would probably be slightly better with List<Birthday> _birthdays = new List<Birthday>. If you want to use IBirthday rather than Birthday, that's fine too, if you use the IBirthday every time you would be inclined to use Birthday except when invoking constructors or static methods. Note that you'll need to be consistent in whether you use Birthday vs IBirthday, and List<T> vs IList<T>.

Note that private members need not adhere to the same rules as public members. If a type were to accept a parameter of type List<T>, that would force outside code to use that type. Switching from List<T> to IList<T> would likely require making many changes in many different assemblies, and make old versions of assemblies incompatible with new ones. By contrast, if a class has a private member of type List<T> which is never shared with outside code, the effects of changing it to use IList<T> would be confined to within the class itself, and would have no effect upon consumers of that class.