An alternative example approach in this case

Question

Hi, I wonder what would be more elegant alternatives to something like this:

class Base... class A extends Base... class B extends Base... //iterator of colection containing mixed As and Bs i want to remowe Bs and do omething with As while(iterator.hasNext()) { Base next = iterator.next(); if(next instanceof A) // do something if(next instanceof B) iterator.remove(); } 

Understand which alternatives ...

Thanks for the tips.

edit: the base class can have many subclasses of not only two, and their number can increase over time

Answer 1

You can create methods in Base and redefine them in A and B

For instance:

 class Base{ public boolean shouldRemove(){ return false; } public void doSomething(){ } } class A extends Base{ @Override public void doSomething() { } } class B extends Base{ @Override public boolean shouldRemove() { return true; } } 

and then you don't need to know which class is an instance of the object:

  while(iterator.hasNext()) { Base next = iterator.next(); if(next.shouldRemove()){ iterator.remove(); } else{ next.doSomething(); } } 

Answer 2

Do you really need to remove them from the list? Why don't you just make a way to do something in the Base class (do nothing) and then just override it for what you want in class A

 class Base{ public void doSomething(){ } } class A extends Base{ @Override public void doSomething(){ // do something } } 

Then you can simply iterate over the list and call the doSomething method for all objects.

 for(Base base : list) { base.doSomething(); } 

Thus, only those classes that redefined the doSomething() method will actually do something. All other classes will simply perform a dummy implementation in the base class.

If Base was an abstract class, you could declare doSomething() as abstract and have its extending classes. With this approach, all classes would have to implement the method and classes for which you do not want any calculations to be performed, you just provided a fictitious implementation of this method. Alternatively, you could even create an interface using the doSomething() method and have (which might even be a better solution) and implement the Base class, given that only existing classes actually implement this method.

Answer 3

I think this is a very short and clear solution and has no alternatives (without raising the code) just add else if instead of if in the second case

You can also split the code into function calls, and if the statement is not huge

Another solution is to create Map delegates that will be called. Like this: interface ISimpleDelegate{ void doSomeLogic(Base b) } `Map delegates = new HashMap ();

After that, add your logic as anonymous classes that implement ISimpleDelegate. delegates.put(A.class, new ISimpleDelegate() { //write your logic here });

I hope the idea is clear.

And in your loop you just call delegates:

 while(iterator.hasNext()) { Base next = iterator.next(); delegates.get(next.getClass()).doSomeLogic(next); } 

Answer 4

instanceof is a good way to filter objects by type - and what you want to do. You have a mixed collection, so you need some kind of filter, either filter the input (do not store anything, but A s) or filter the output (do not process anything except A s).

If you just don't like "instanceof", you can use enum to specify the type and add the final method to get the type in Base :

 enum Type { ATYPE, BTYPE }; 

---

 public Base { final private Type type; public Base(Type type) { this.type = type; } public Type getType() { return type; } // ... } 

---

 public A { public A() { super(Type.ATYPE); } } 

---

 while(iterator.hasNext()) { Base next = iterator.next(); switch (next.getType) { case ATYPE: // do something and break case BTYPE: iterator.remove(next); break; } } 

Answer 5

In general, a good solution to avoid instanceof is to use a so-called visitor pattern .

For this template, you need an additional interface (visitor), its implementation containing the code you want to execute, and an additional method in all classes of your hierarchy, so this can be excessive in small cases (but it is very convenient if there is not only A and B , but also more types).

In your case, it will look like this:

 interface Visitor { void visit(A a); void visit(B b); } class Base { abstract accept(Visitor v); } class A extends Base { accept(Visitor v) { v.visit(this); } } class B extends Base { accept(Visitor v) { v.visit(this); } } class MyVisitor implements Visitor { visit(A a) { doSomethingWithA(a); } visit(B b) { doSomethingWithB(b); } } 

It is used as follows:

 MyVisitor v = new MyVisitor(); while(iterator.hasNext()) { Base next = iterator.next(); next.accept(v); } 

The advantage is that you should write most of the code only once. If you want to do other things with A and B elsewhere in your program, just write a different version of Visitor. You do not need to modify Base , A and B in the same way as if you added doSomething() to these classes.

Edit: If the number of subclasses increases, you need to change all existing Visitor implementations. However, at least the compiler talks about this. With instanceof you can forget the place where you need to add a processing clause. This may not be detected at run time, while the visitor pattern gives you security at compile time.