Discussion on: SOLID: Liskov Substitution Principle

[Mihail Malo]

These abstract classes sure sound like traits/interfaces.

[Victor Manuel Pinzon]

LSP was originally defined under the context of subtyping, but sure, it can also be applied under the context of traits/interfaces.

[Mihail Malo]

I just don't understand what we achieve by enforcing the WithdrawableAccount extends BankAccount relationship. Seems like two independent traits would be fine here?

const ADMINISTRATIVE_EXPENSES: f64 = 25.0;
fn main() {
    let mut basic = BasicAccount::default();
    let mut premium = PremiumAccount::default();
    for (amt, acc) in (1..)
        .map(|x| x as f64 * 100.0)
        .zip([&mut basic as &mut dyn Deposit, &mut premium])
    {
        acc.deposit(amt);
    }
    for acc in [&mut basic as &mut dyn Withdraw, &mut premium] {
        acc.withdraw(ADMINISTRATIVE_EXPENSES);
    }
    println!("{:#?}", [&basic as &dyn core::fmt::Debug, &premium]);
}
trait Deposit {
    fn deposit(&mut self, amount: f64);
}
trait Withdraw {
    fn withdraw(&mut self, amount: f64) -> bool;
}
#[derive(Debug, Default)]
struct BasicAccount {
    balance: f64,
}
impl Deposit for BasicAccount {
    fn deposit(&mut self, amount: f64) {
        self.balance += amount;
    }
}
impl Withdraw for BasicAccount {
    fn withdraw(&mut self, amount: f64) -> bool {
        if self.balance < amount {
            return false;
        }
        self.balance -= amount;
        true
    }
}
#[derive(Debug, Default)]
struct PremiumAccount {
    balance: f64,
    preference_points: u32,
}
impl PremiumAccount {
    fn accumulate_preference_points(&mut self) {
        self.preference_points += 1;
    }
}
impl Deposit for PremiumAccount {
    fn deposit(&mut self, amount: f64) {
        self.balance += amount;
        self.accumulate_preference_points();
    }
}
impl Withdraw for PremiumAccount {
    fn withdraw(&mut self, amount: f64) -> bool {
        if self.balance < amount {
            return false;
        }
        self.balance -= amount;
        self.accumulate_preference_points();
        true
    }
}

Look at it go!

[Victor Manuel Pinzon]

Yes, it'd work but your code wouldn't be enforcing a bank account behavior. Remember, SOLID principles help to make OOP easier and one of the basic principles of OOP is to model real-world objects into programming objects. On the other hand, LSP helps you to model behaviors instead of just properties by using subtyping, is-a relationship between objects. What you achieve by setting with "WithdrawableAccount extends BankAccount" is to ensure that "WithdrawableAccount" extends whatever the behavior defined in the BankAccount class.

In your example, though I'm not very familiar with Rust lang, you defined independent traits which you implement as you need in your different structures but your code is not enforcing any relationship between bank account types . Suppose that BankAccount abstract class declares ten more different behaviors and not just deposit, same with WithdrawableAccount. Now, you need to implement a different type of bank account, if you wanted to have the behavior defined in the previous classes you would need to declare all the independent traits in your new type of class. What would happen if you forget to implement one of those traits? What would happen if a workmate is doing the implementation instead of you, would he/she know which traits to implement? This happens because there is no relationship enforcing in your code. On the other hand, in my example, by just extending the "WithdrawableAccount" type, I'm making sure that my new code extends whatever the"WithdrawableAccount" behavior and whatever the "BankAccount" behaviuour, and this is achieved because there is a hierarchical "is-a" relationship.

Hope this helps you.

[Mihail Malo]

But what if you want to have an account type where you can withdraw() but not deposit()?
In the article, you break out the Withdrawable behavior from BankAccount.
But I don't see how deposit() is the more fundamental method than withdraw(), so I broke out both.
Perhaps in a real architecture, you'd always have both methods, and they would be fallible.

trait Account {
    fn deposit(&mut self, amount: Money) -> Result<(), DepositError> {
        Err(DepositError::OperationUnsupported)
    }
    fn withdraw(&mut self, amount: Money) -> Result<(), WithdrawError> {
        Err(WithdrawError::OperationUnsupported)
    }
}

(You would probably forego default implementation, to ensure implementers consider implementing both methods, but this is approximately what the implementation would be if the operation is not supported on a particular account type)
I understand you could do a bunch of runtime reflection in Java, but in the article you only did the negative check instanceof LongTermAccount to skip some, not check if withdrawals are positively supported.

However if it was the intention that LongTermAccounts don't make it into the list at all, the separate trait solution works.

[Victor Manuel Pinzon]

But what if you want to have an account type where you can withdraw() but not deposit()?

I don't know of any type of bank account that allows withdrawals but no deposits. That's why in the post's example I designed the classes with the deposit method as the fundamental method in all bank account. Now, let's suppose that some crazy dude from product development department asks you to implement a bank account type that allows only withdrawals (even when I don't see any business case there), in that case, yes it is better to have separate interfaces and that separation is pretty much the "interface segregation principle" which is also part of the SOLID principles.

I understand you could do a bunch of runtime reflection in Java, but in the article you only did the negative check instanceof LongTermAccount to skip some, not check if withdrawals are positively supported.

Yes, you're right, I could've used reflection in order to validate if the class supported the withdraw method. But remember, reflection is mostly used when we want to inspect a class during runtime and we don't know the class specification or we don't have access to the code. My question is, why would you want to enforce the "withdrawal" validation during runtime using reflection, when you have full access to the code and you could easily enforce that during compilation time by changing the design of your classes?

Remember, SOLID principles are just guidelines not rules. I'm not saying that your independent trait solution is wrong, I'm just saying that is not LSP compliant.

[Stepan Mozyra]

Yes, Mihail, this is because of not really good example. Abstract class in this article not an abstract class at all, but an interface.

Real problem occurs when you have some implementation in your base class, but then overrides it in ancestor. This what LSP is actually about. (And there's just a simple rule to prevent such a violation)

[Mihail Malo]

Bruuuh inheritance is scary lmao