Two Laptop Bag*
The moose likes Java in General and the fly likes Removing multiple if conditions Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Java in General
Bookmark "Removing multiple if conditions" Watch "Removing multiple if conditions" New topic
Author

Removing multiple if conditions

Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Hello everyone,

I have a sample java code where in there are as many as 30 if conditions which are checked for each line item :

Example:

if(product.equals("A"))
{
//do something
}

if(poduct.equals("B"))
{
//do something
}

----- so on

Whenever a new line item is added i am supposed to add this as a new if conditional check. This makes the code messy and should not be encouraged.So can anyone suggest me what can be done so as to remove these if conditions. Cant we have a single method which can be reused to do achive the same purpose.

Any help will be appreciatd.
Thanks in advance,
Saurabh


Success is not doing extraordinary things but doing ordinary things extraordinarily well.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
You could try reducing your A B C to chars and using a switch block.

Or you could try designing your products. What you have is something in your catalogue class doing actions with different products. If there is a different action with each different product, you ought to have the actions programmed polymorphically into the different product classes.
NOT:
BUT:
etc etc.

CR
[ May 28, 2006: Message edited by: Campbell Ritchie ]
Tony Morris
Ranch Hand

Joined: Sep 24, 2003
Posts: 1608
Google for "strategy design pattern" - not that I approve of such euphemisms, it is actually the correct approach for you to take in this circumstance (my assumptions withstanding).


Tony Morris
Java Q&A (FAQ, Trivia)
Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Originally posted by Campbell Ritchie:
You could try reducing your A B C to chars and using a switch block.

Or you could try designing your products. What you have is something in your catalogue class doing actions with different products. If there is a different action with each different product, you ought to have the actions programmed polymorphically into the different product classes.
NOT:
BUT:
etc etc.

CR

[ May 28, 2006: Message edited by: Campbell Ritchie ]


Hello Campbell,

Thanks for the reply. First of all i dont want to use switch since again its going to add lot of code for each product. The other approach as you said seems to create a different class for each product, which essentially means that whenever a new product is added a new class needs to be created. So my task of reducing code and providing a flexible solution is not completed.

Please let me know what can be done if you have something other than this in mind.

Saurabh
Mandar Max
Ranch Hand

Joined: Mar 14, 2006
Posts: 38
Saurabh, I guess Campbell's solution is better and more flexible. The reason for this is, when you want to add more products you will not have to change existing code. This will not only make the code cleaner but also more maintainable. Probably you should review your goal of reducing the number of lines of code...


"The trouble with doing something right the first time is that nobody appreciates how difficult it was!"
Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Originally posted by Mandar Max:
Saurabh, I guess Campbell's solution is better and more flexible. The reason for this is, when you want to add more products you will not have to change existing code. This will not only make the code cleaner but also more maintainable. Probably you should review your goal of reducing the number of lines of code...


Yeah i guess it does make sense. Here is my code: Can you please tell me how to go about it:

if (product.equalsIgnoreCase("Courier")) {
x = x + lineitem.amount;
} else if (product.equalsIgnoreCase("Insurance")) {
y= y+ lineitem.amount;
}
....
....

else{
// do something
}

How to design classes for it. my product is a string object.

Saurabh
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
Thank you.
If you have different data or fields or descriptions associated with each product, then you still want the "polymorphic" approach, but your method will be all the same.
Try this sort of thing:-




CR
Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Hi Campbell,

The example was quite useful but still i have one confusion.

Suppose i have the following piece of code:

public abstract class Product{

private String productType;

public abstract int calculate();

}

public class Book extends Product{

public int calculate()
{
//some code 1
}
}

public class Pen extends Product{

public int calculate()
{

//some code 2
}
}


Now when i do something of this sort:

Product product= new Product();

product.calculate();

Then which calculate() method will be called.. will that be of book or pen's method ??

My method call will depend on the productType field in product., So i am little confused.

Please let me know.

Saurabh
Garrett Rowe
Ranch Hand

Joined: Jan 17, 2006
Posts: 1296
Now when i do something of this sort:

Product product= new Product();

product.calculate();

Then which calculate() method will be called.


You can't actually do that. Abstract classes cannot be instantiated directly. what you can do is say:


Some problems are so complex that you have to be highly intelligent and well informed just to be undecided about them. - Laurence J. Peter
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
Only too glad to help.

Please check my last posting; I might have been mistaken about {} in the method calls in the go() method; if you have any problems you can leave the {} out.

Enhancement suggested: Rather than "Screwdriver extends Product," how about this:-NOW: you can have a HashMap<String, Integer> shoppingListMap, and put subtotals for your categories on your shopping list in it. Then you can update each of the Integers.

Try a new version of the go() method:-See whether that works; I haven't tried it myself. If you are feeling really clever. create an enum with the product categories in.

You can't write because Product is quite correctly abstract, and the compiler will tell you off.
You can write Then the calculate() method from Pen will be called. It's called dynamic binding; even though you have Pen, Book, Screwdriver, BottleOfMilk etc classes which inherit from Product, you get the Pen method from a Pen, the Book methdo with a Book, the Screwdriver method with Vodka and Orange oops Screwdriver, etc.

CR
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
And why does your method in the class depend on some other datum in the class? If you have a different calculate method for a Pen or a Book, then don't you you simply write the code in the method?
Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Hi guys,

Thanks a lot for your response. Now i want to ask you one more thing on the same discussion, Carring forward the same piece of code:
Suppose i have the following piece of code:

public abstract class Product{

private String productType;

public abstract int calculate();

}

public class Book extends Product{

public int calculate()
{
//some code 1
}
}

public class Pen extends Product{

public int calculate()
{

//some code 2
}
}


Now when i do something of this sort:

Product product= new Book();

product.calculate(); // will call calculate of book

Product product= new Pen();

product.calculate(); / will call calculate of pen.

Now if i have a list of products such that i need to iterate on them and depending on type of product call appropriate calculate method:

It goes like this:

for(int i = 0; i < lineItems.size(); i++)
{
Product product= (Product ) lineItems.elementAt(i);

String type= product.productType;

if(type.equals("Book"))
{
book.calculate();
}else if ()
{

}.. so on

But this will again mean the same if conditions which doesnt make sense of using the Strategy design pattern.

How can i remove these if conditions and call calcualte based on product type:


if i write:

Product product= new Book();
product.calculate();

Product product= new Pen();
product.calculate();
.... so on

this inside a loop doesnt make any sense .. so i am not reaching onto a logical conculsion.

Any help?

Saurabh






}




}
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
No, you have missed the point of polymorphism. You have a method in Product with a signature; I think in this case it was a concrete method. So every subclass of Product has that same method, unless you override it. Overriding in this context means producing another method with the same signature, but different contents. And remember you can't override a "public" method to become "private."

That means that in every subclass of Product, you have the choice of using the same method (but with different data), OR using a new method.

Now you have three classes I have written out, Book, Screwdriver and BookWithCD. You ignore and leave out the if altogether. The JVM will work out whenever it goes through your different classes whether you have a Book, a Screwdriver, or a BookWithCD, and will choose the appropriate method for each.
And there are more elegant ways of going through all your shopping list.
1: One way to do it2: As you can see, the array which Garrett Rowe suggested works much better.

3: What I would prefer, using an ArrayList. If you use Java 5.0, you don't need the cast.

Polymorphism implies that you delegate calculating different prices to the different Products, and your Shop class can use exactly the same method call to get the prices including and excluding tax.
No if blocks or anything required.

CR
[ May 29, 2006: Message edited by: Campbell Ritchie ]
Saurabh Agrawal
Ranch Hand

Joined: Oct 07, 2003
Posts: 244
Hi CR,

I got your point but i have one issue in implementing this. In your example you are creating an arraylist and adding differnt products to it. At the end u r interating over it to calculate sum of all the products.

In my case --1) List if prepopulated with values

2) I need to take each value and based on the type of product i need to calculate the total vat for each product.

3) calculation rules will be different for each product.

Keeping above things in mind if i follow his approach:

for(int i=0; i < list.size ; i++)
{

Product product=(Product)list.get(i);

// if product.productype = book then i need to call calculate of book

// if product.productype = pen the i need to calculate of pen

.... so on


}

So how is it possible to call this in loop when i am not sure the product ia m having will be a book or pen.

Henceforth, i cant do something like this

Product product= new Book();
product.calculate()

Product product= new Pen();
product.calculate()

since this will be in loop and we are not sure what is the item.

So what can be done in this case??

Saurabh


}
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
Try what I told you. It works. I have spent 3/4 hours coding it and it works.
I tried with six subclasses and got this print-out:-
polymorphic.Book Hard Times Charles Dickens 1854 Penguin Books Ltd
Price: �2.99, VAT rate: 0.0%, VAT exempt: false
VAT in pence: 0
polymorphic.BookWithCD Java How to Program 6th Edition Deitel and Deitel 2005 Prentice-Hall
Price: �43.99, VAT rate: 17.5%, VAT exempt: false
VAT in pence: 174
polymorphic.BottleOfMilk pasteurised semi-skimmed
Price: �0.45, VAT rate: 17.5%, VAT exempt: true
VAT in pence: 0
polymorphic.BeefJoint Topside 1lb 8oz
Price: �9.99, VAT rate: 17.5%, VAT exempt: true
VAT in pence: 0
polymorphic.Saw Eclipse Hard-point 10"
Price: �9.99, VAT rate: 17.5%, VAT exempt: false
VAT in pence: 174
polymorphic.Screwdriver Cross-head Black and White 6" Spear & Jackson
Price: �4.99, VAT rate: 17.5%, VAT exempt: false
VAT in pence: 87
For the BookWithCD I put down CDPrice = 999, which give VAT = 174, but I haven't printed out the CD price separately.
Stan James
(instanceof Sidekick)
Ranch Hand

Joined: Jan 29, 2003
Posts: 8791
So how is it possible to call this in loop when i am not sure the product ia m having will be a book or pen.


This is precisely the value of polymorphism. You don't have to know. And if somebody adds a new product type you still don't have to know.

The type of Product assures us that any object in the array will have the calculate() method. You can use a Product reference to every one of them and call the calculate() method even if they are really a mix of Pen, Book and Dishwasher objects. Different objects in the array can do different calculations inside that method.

This is some very cool stuff. Your sumOfValues method is "closed" for modification but "open" for extension, meaning you can extend the program to use new products without ever touching the sumOfValues method. Imagine a big company where dozens of programmers are developing new products every day. They don't have to compete to check out sumOfValues and modify it for every new product. And you don't have to test it again!

Each Product class like Book becomes the one clear place to put any special behavior for a Book. Again imagine that big company ... you wouldn't want to keep track of "if it's a book ..." in dozens of classes if you could put all the book code in one place.

Lemme know if that helps!


A good question is never answered. It is not a bolt to be tightened into place but a seed to be planted and to bear more seed toward the hope of greening the landscape of the idea. John Ciardi
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39828
    
  28
Go and find a Java book of any size, find the chapter about polymorphism and inheritance and read it.
Go and ask whoever is teaching you programming about this problem.
Go to this website and download the text for Thinking In Java by Bruce Eckels. [Remember it might not be the most recent edition.] Read the chapter about polymorphism and inheritance (Ch 7).
Go to The part of the Java tutorial about inheritance. Read it. Unfortunately it doesn't actually mention polymorphism.
We have told you.
  • No if statements
  • No checking class of the object
  • No casts
  • All you have to do is put the method in the superclass, and call it from any object which is an instance of that class.
    As Stan James told you, just write
    As long as the method signature ie method name and types of arguments, are the same, you can call the same method from every subclass.
  • It will work if all the methods are the same.
  • It will work if most of the methods are the same and a few are different.
  • It will work if the methods are the same but the data are different.
  • It will work if the methods are all different.
  • You just writeIt will work. I promise. I have shown you how you can have a shopping spree and by different articles with different types of tax on them and it works them all out differently because different methods or different data are used in the calculation.

    [But it might be easier if you reduce the number of abstract classes. I think I suggested too many. Sorry.]

    Find a simple example of polymorphism in one of the books. Copy it out. Copy the code by hand, not copy-and-paste. Run it. See that it works.

    CR
    Saurabh Agrawal
    Ranch Hand

    Joined: Oct 07, 2003
    Posts: 244
    I have the following classes:

    1) OrderLineItemData.java which is super class(same as product)

    public class OrderLineItemData {
    public static String supplier;

    public static String orderId;

    public static double amount;

    public static double vatAmount;

    public static double supplierFee;

    public static int lineId;

    public static String product;

    public static String productId;

    public String netPayable;

    public void calculate() {
    // do some calculation
    }
    }

    2) Insurance.java

    public class Insurance extends OrderLineItemData {

    private double totalInsurance = 0.0;

    private double totalAIGCost = 0.0;

    private double totalCogs = 0.0;

    public void calculate() {
    totalInsurance = totalInsurance + OrderLineItemData.amount;
    totalAIGCost = totalAIGCost+ (OrderLineItemData.supplierFee + OrderLineItemData.vatAmount);
    // the totalCOGS amount will always be the opposite
    // value of supplierFee
    totalCogs = totalCogs+ ((OrderLineItemData.supplierFee + OrderLineItemData.vatAmount));
    }
    }

    3) Test.java


    public static void main(String args[])
    {

    Vector lineItems= somemethod.getLineItems();

    for (int i = 0; i < lineItems.size(); i++) {
    OrderLineItemData lineitem = (OrderLineItemData) lineItems.elementAt(i);

    // What should i code here to call product specific calculate i.e. if prouduct type in lineitem.product is insurance then calculate of insurance shoulld be called and if product type is car then calculate of car should be called.


    /* This doesnt seem to be okay ??
    lineitem= new Insurance();
    lineitem.calculate();
    */

    }
    Saurabh Agrawal
    Ranch Hand

    Joined: Oct 07, 2003
    Posts: 244
    Hi CR,

    The problem in my case is that my list of products have same type of objects i.e orderlineitems. These similar objects are just distinguished from each other by an attribute named product which can be car, insurance,etc.

    So when i iterate through all these items, similar objects will be available and henceforth if we call calculate method then calculate of same object i.e. orderlineitem will be called.

    I got the concept of polymorphism which says if my list is having different product i.e. different objects then method of each object will be called seperately dependingh upon object.

    Since i am working on a legacy code whose design is such that instead of having seperate objects they have created vector of similar objects distinguised from each other by just a field name, henceforth applying polymorphism wont be possible.

    Any thoughts around my problem ? Are you able to understand my problem now.

    I think the only possible solution which i see here is to change the design and seperate all objects and then add the list to apply polymorphic behaviour to the class.

    Saurabh
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39828
        
      28
    Are you able to understand my problem now.

    I think so, yes. Somebody has dropped you in it.

    Try:
  • Some sort of HashMap whereby you use the order code as a String, and some other sort of object as the "V" part to hold your subtotal.
  • Put details into your objects eg rate of VAT, which use the same calculate method. Different data, same method=different result.
  • Getting rid of the "static" before each of the attributes for your classes.
  • Design the item classes properly.

  • I think you will find you save time by scrapping what looks like a dreadful design altogether.

    Good luck

    CR
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39828
        
      28
    I did think about a Singleton object for each of your categories which carries the subtotals.

    I think that would make things even more difficult than at present.

    CR
    Stan James
    (instanceof Sidekick)
    Ranch Hand

    Joined: Jan 29, 2003
    Posts: 8791
    Tony mentioned Strategy way up the page. It's one hammer that I tend to pull out fairly often.

    Move the special code for each product type into a little Strategy class that does the work on a product argument. Find the right Strategy based on the product name and execute it. Your code looks like:

    You'll have to imagine the BookStrategy, ToasterStrategy, TroutStrategy, etc. all put into the map at startup.

    Does that solve the right problem?
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39828
        
      28
    BookStrategy, ToasterStrategy, TroutStrategy, etc. all put into the map at startup.

    Stan, how do you do that? Can you set up an enum like an array and go through all its members putting them into the map?
    Or do you have to do it with each member individually?

    CR
    Stan James
    (instanceof Sidekick)
    Ranch Hand

    Joined: Jan 29, 2003
    Posts: 8791
    I guess I've usually done this two ways at startup ...

    For each strategy in the configuration, create a single instance and put it in the map. Return the single instance any time somebody asks for an instance.

    For each strategy in the configuration, put the classname in the map. Create & return a new instance any time somebody asks for an instance. Actually, you might skip any startup processing and just get the classname from config every time.

    And I've done a third that doesn't need any configuration. I don't think I'd recommend this one, but it shows how many ways you can skin a cat.
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39828
        
      28
    Thank you.

    . . . and can you get a list of "Products" at runtime to put into that factory method?

    And haven't we hijacked Saurabh Agrawal's thread nicely!

    CR
    sinasi susam
    Ranch Hand

    Joined: Jul 15, 2005
    Posts: 67
    hi all,
    suppose that all codes in if statements are in different methods.

    How about using reflection to invoke different methods instead if stattements in Runtime?

    for example ;


    good luck.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Do not use reflection. Ever. (unless you are developing development tools).

    This problem calls for the strategy pattern. Figure it out. Apply it.
    sinasi susam
    Ranch Hand

    Joined: Jul 15, 2005
    Posts: 67
    "String classname = "com.whatever.strategies.StrategyFor" + product;
    Strategy = Class.forName( className ).newInstance();
    "

    why? is this something more different than refl?

    why mustn't we?
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Originally posted by sinasi susam:
    "String classname = "com.whatever.strategies.StrategyFor" + product;
    Strategy = Class.forName( className ).newInstance();
    "

    why? is this something more different than refl?

    why mustn't we?


    Yes, thats quite a bit different than reflection. I wouldn't use that technique for a strategy pattern either though. Why not just instantiate all the objects and add them to a map. There is no reason to hide the loading of the class in this way. You have to create a new class anyway.

    If you are trying to not have to modify your strategy section when you add a new class I don't think this will work. If the string passed in does not point to the proper class then you get ClassNotFoundException which is easily avoided. I suppose it depends on how much independence you want. It comes at a cost.
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Removing multiple if conditions