• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

how to use instanceOf?

 
Ranch Hand
Posts: 64
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I have an interface Discount and there are many types of discount as shown below:-
interface Discount
Abstract Class DiscountA , Class DiscountB
Class DiscountA1, Class Discount A2
Then I have a product class which has a list of discounts, we can multiple discounts of same type or diff type.



Now I have to find the discount value by DIscountA1, or By DiscountA2 etc so I have implemented the below method in PRODUCT class:-



Now the question is, is it right to use instanceOf operator here?

One options that I considered is:-
1) Create an Enum DiscountType{DiscountA, DiscountA1, DiscountA2, DiscountB....}
Modify the method as below:-

Have getType in Discount and all the classes will override it, example:-




Which is the right way to do it and why, also can there be a better design?
I was leaning towards using instanceOf as I think creating an enum of discountType is just extra work.
 
Bartender
Posts: 3648
16
Android Mac OS X Firefox Browser Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Your Discount is an interface. Do DiscountA and DiscountB abstract classes "implement Discount"? Are DiscountA1, DiscountA2 concrete classes?

If I understand your design correctly then I would probably use a Map<Discount, Integer> storing the different discount values rather than using Enum or checking using instanceof operator.

The key to the map may need some thinking. Will say Class<Discount> be a more relevant key than just Discount? It really depends on how you plan to store such values.

Edit: for maintenance purposes, code will often change if using Enum or instanceof when there is a new type is added or need removing.
 
andy kumar
Ranch Hand
Posts: 64
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Discount is an interface.
DiscountA is an abstract class that implements Discount
DiscountA1 and DiscountA2 are concrete classes that extend DiscountA

DiscountB is a concrete class that implements Discount
 
andy kumar
Ranch Hand
Posts: 64
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
any more input from the experts, I am still leaning to instanceOf.
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

andy kumar wrote:
Now the question is, is it right to use instanceOf operator here?
...
Which is the right way to do it and why, also can there be a better design?
I was leaning towards using instanceOf as I think creating an enum of discountType is just extra work.



This is absolutely NOT a proper use of both interfaces and the instanceof operator.

When you program to an interface, you should not have to use instanceof. If you find yourself doing so, then that is a Code Smell. Creating an enumerated type such as DiscountType would make the design even more "smelly". The interface type should be the only type you should be concerned with from a usage standpoint. That is, any code that is concerned with a Discount should not have to care whether it's a particular kind of discount. If you find yourself writing code that does need to distinguish between different implementations of an interface, then you have subverted the intent of the interface and there is most likely something wrong with your design.

The code you gave does not reveal much about the underlying problem that you're trying to solve but my gut reaction is that you have way too much abstraction. You can probably eliminate the abstract class DiscountA. Why do you think you need abstract class DiscountA?
 
andy kumar
Ranch Hand
Posts: 64
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The Abstract class DiscountA was just an example, I can surely get rid of it, so now I have a discount interface and a bunch of implementations of this class, DiscountA DiscountB DiscountC etc
The underlying problem is that I am just trying to construct a data model for the following:- Have a product which has a list of discount associated with it, the product can have multiple discount of the same type. now the client using the product should be able to get sum of discounts by type, so for example the client wants the total discount on the product by DiscountA.

Another solution would be to have an method getTotalDIscountA in the Discount class and then DiscountA will override it. But then DiscountB should also override it and probably return a zero amount. Is this a better solution? So in summary Discount will have methods like getTotalDIscountA getTotalDIscountB getTotalDIscountC and each concrete class will implement these methods, only one of the methods apllies to each class and the other methods will return zero. Or I can make Discount an abstract class and provide deafult implementation of this methods and then the concrete class will override the approprite method.
Is this a better solution compared to the enum type stuff.


 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Andy,

It's hard to understand the problem if all you have are names like DiscountA, DiscountB, DiscountC. These have no meaning and no relevance to any kind of context. Why are there multiple discounts applicable to a product? Under what circumstances would any particular discount be zero? Why do you care if it's A, B, or C? Isn't it just the value of the discount that really matters for the calculation, not the A, B, or C designation? Could there be a further discounts applicable, such as (for lack of a better name) DiscountD?

It is ALWAYS wrong for an interface or an abstract class to have any kind of reference to one or more specific implementations. The point of defining a type hierarchy is to generalize as you go up the hierarchy (to the parents) and to specialize as you go down the hierarchy (to the children). If a high level type has specific knowledge of lower level types, that is a wrong design.

What if instead of getTotalDiscountWhatever(), you had an applyTo() method? That is:



Then you could just have a list of Discount implementations over which you can iterate and call each applyTo() method.


The Product class should not be aware that there are different implementations of Discount involved. All it should know is that a number of Discounts can be applied to an amount.
 
andy kumar
Ranch Hand
Posts: 64
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Junilu,

First of all I sincerely appreciate your response.
Ok more realistic discounts are promotionalDiscount, couponDiscount, frequentlyBoughtTogetherDiscount. Now in promotionalDiscount we could have shippingPromotionalDiscount or itemPromotionalDiscount or orderPromotionalDiscount but for simplicity lets not consider that, so we just have promotionalDiscount, couponDiscount, frequentlyBoughtTogetherDiscount.

And a product can have all 3 discounts at a time. so lets assume a product like computer table worth $150 which has promotionalDiscount $10 , couponDiscount $15 and frequentlyBoughtTogetherDiscount $20 so total discount is $45.

So first step is to determine all the discounts and lets say in this scenario all the 3 discount apply, so I calculate the discount amount create the discount object and insert it into the product.

Now based on business rules we can apply
1) all the discount or
2) we have to find the best of discount between promotionalDiscount & couponDiscount and give the better one and always give frequentlyBoughtTogetherDiscount
3) in some scenarios we find the best of the 3 discounts

So I have a service class that takes the product as an argument, this class needs to have a knowledge of the individual type of discounts as it has to make a decision to find the better discounts. It has comples business logic to decide which discount applies and which does not.

Could there be a further discounts applicable, such as (for lack of a better name) DiscountD?
Yes I assume we could have different kinds of discounts in the future.

So my previous suggestion was like this:-






But I don't like this at all because as the types of discount increase each of the concrete classes have to implement the extra unnecessary method, I hope you are getting what I mean.

The Product class should not be aware that there are different implementations of Discount involved. All it should know is that a number of Discounts can be applied to an amount.
I agree and that I what I want to achieve.

May be I can keep the product class simple and it just has a list of discounts, but then the service class(which gets this product) needs to somehow have a knowledge of the concrete discount values so that it can make an appropriate decision
OR
I should not insert the discounts in the product in the first place, the service gets the product as an argument finds all the applicable discounts (thus it will naturally know the concrete discount classes) and then add only the appropriate discounts to the product....) If I do this I can completely get rid of instanceOf. Example:-

 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

andy kumar wrote:Junilu,

First of all I sincerely appreciate your response.


No problem


Ok more realistic discounts are promotionalDiscount, couponDiscount, frequentlyBoughtTogetherDiscount. Now in promotionalDiscount we could have shippingPromotionalDiscount or itemPromotionalDiscount or orderPromotionalDiscount but for simplicity lets not consider that, so we just have promotionalDiscount, couponDiscount, frequentlyBoughtTogetherDiscount.

I don't really see how the second set would be simpler but Ok, if you say so...


And a product can have all 3 discounts at a time. so lets assume a product like computer table worth $150 which has promotionalDiscount $10 , couponDiscount $15 and frequentlyBoughtTogetherDiscount $20 so total discount is $45.

So first step is to determine all the discounts and lets say in this scenario all the 3 discount apply, so I calculate the discount amount create the discount object and insert it into the product.

Ok, I'm still with you, except for the statement "insert it into the product" which doesn't make a lot of sense to me if I were "thinking objects" -- "insert into the product" smells more like procedural type thinking. There's nothing wrong with procedural thinking per se, but you must be careful not to mix your paradigms inappropriately. If you're going to mix paradigms, I would say mix functional and object thinking. That seems to work better than procedural and object thinking.


Now based on business rules we can apply
1) all the discount or
2) we have to find the best of discount between promotionalDiscount & couponDiscount and give the better one and always give frequentlyBoughtTogetherDiscount
3) in some scenarios we find the best of the 3 discounts


Here, it seems to me you have to look at the Strategy Pattern. Each item in that list is a different strategy: CumulativeDiscountCalculation (add all applicable together), BestDiscountCalculation, MultiDiscountCalculation. Each strategy encapsulates the "rules" for applying the discount.


So I have a service class that takes the product as an argument, this class needs to have a knowledge of the individual type of discounts as it has to make a decision to find the better discounts. It has comples business logic to decide which discount applies and which does not.


The discount strategy classes I mentioned above would be your "business classes" or "domain objects" -- I use the term "service classes" differently: they provide functionality such as data access, access to mail gateways, message queues, web services, etc. Think of service classes as enablers -- things that are tied closely to the architecture of the system. They usually go down to JVM-level and OS-level service layers. They are not really related to the actual business problem but rather they help to get things or allow things to get done.


So my previous suggestion was like this:-


In a code review, I would mark this code with a BIG RED FLAG. Again, you have mixed specifics with abstraction. Make sure to keep specific implementations sufficiently decoupled from the abstract notion of intent. The first sign that something is wrong with this is that there are only getter methods. Getter methods only serve to expose information. They have very little, if anything, to do with BEHAVIOR, which is what an interface should be most concerned with defining: the general behavior of any object that implements the interface.




But I don't like this at all because as the types of discount increase each of the concrete classes have to implement the extra unnecessary method, I hope you are getting what I mean.


It's good that you don't like this because you need to throw this idea out and forget about it. It's a poor design.


May be I can keep the product class simple and it just has a list of discounts, but then the service class(which gets this product) needs to somehow have a knowledge of the concrete discount values so that it can make an appropriate decision

There is nothing wrong with this thought except the naming -- the DiscountStrategy classes I mention above are the "domain/business classes", not service classes.


OR
I should not insert the discounts in the product in the first place, the service gets the product as an argument finds all the applicable discounts (thus it will naturally know the concrete discount classes) and then add only the appropriate discounts to the product....) If I do this I can completely get rid of instanceOf.


You're headed in the right direction now. Again, it's a BUSINESS or DOMAIN class, not a Service class that needs to know about concrete discount classes.




Almost, IMO. I would suggest this:

DiscountFinder -- Given a Product, the DiscountFinder can give you a List of Discounts that apply to that Product, via the findApplicableDiscounts() method

DiscountCalculator - encapsulates a particular strategy for applying discounts. Give a DiscountCalculator implementation a Product and a list of Discounts and it will apply a specific algorithm for getting the effective discount for that Product. Possible implementations: AllDiscountCalculator, BestDiscountCalculator, SpecialDiscountCalculator, LoyaltyDiscountCalculator.

Discount - Knows how to apply a discount to an amount. It can be as simple as just subtracting a fixed value (e.g. a Coupon) Or it can calculate a percentage (MarkdownDiscount) or whatever.

DiscountingService - coordinates flow of information between DiscountFinder, DiscountCalculator, and Discount objects.

Caveat: These are all just off the top of my head. At this point, if I were developing this, I would stop talking/discussing for a while and start writing some tests and do TDD.

Good luck.
 
Hey! Wanna see my flashlight? It looks like this tiny ad:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic