K. Tsang CEng MBCS PMP PMI-ACP OCMJEA OCPJP
K. Tsang wrote:The first thing I noticed is you have "a customer in an account". I would consider the other way round - a customer has an account.
For the account, I actually would use an interface rather than abstract class. Then have the CheckingAccount and SavingsAccount implement the getInterestRate() method.
Junilu Lacar wrote:I don't see how any of these suggestions address the design issues that the OP asked about. How does reversing ownership roles help prevent violating the OCP if a new Customer or Account type is introduced?
While programming to interfaces is a fine approach in and of itself, in this case I don't see anything wrong with the choice of using an abstract class, at least not with the given code. The abstract class actually establishes that there is a relationship between an Account and Customer. If you were to start with an interface, you would have to establish the relationship some other way.
K. Tsang CEng MBCS PMP PMI-ACP OCMJEA OCPJP
K. Tsang wrote:The relationship between Account and Customer is just a bit odd for me at first, depending on which way you look at it. Customer has Account or Account belongs to Customer... means the same thing.
Junilu Lacar wrote:The red flag is your use of instanceof - that's what causes you to break the open-closed principle. This means that something else other than the Account class should be responsible for "knowing" what the interest rate is. From your current logic though, it appears that the combination of Account and Customer is what determines the interest rate. The first data structure that comes to mind is a Map. An InterestTable object is what I might try. This class is responsible for knowing/calculating the interest rate given an Account and Customer.
If you go with this and you add a new customer or account type in the future, you would extend the InterestTable class, or simply "register" the new Customer, Account, and interest rates. Nothing else would need to change.
K. Tsang wrote:The first thing I noticed is you have "a customer in an account". I would consider the other way round - a customer has an account.
For the account, I actually would use an interface rather than abstract class. Then have the CheckingAccount and SavingsAccount implement the getInterestRate() method.
Mandy Ram wrote:I can make account as interface and remove customer reference from Account and have one abstract class implementing the interface and which has customer reference and can then make my Savings and CheckingsAccount classes extend that abstract class.That can be done.
I am actually looking for other changes which can make the design better in terms of extensibility.
"Leadership is nature's way of removing morons from the productive flow" - Dogbert
Articles by Winston can be found here
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime. |