CODEWITHSUNDEEP
javaoopenums
membersound
membersound

Reputation: 86747

Is instanceof a good practice?

I have a list of objects which extend from a base class. Now I want to apply a specific operation only on one instance of classes in the list.

Is the use of instanceof a good practice there? Or should I rather differ the objects by eg a custom enum?

abstract class Base;
class Foo extends Base;
class Bar extends Base;

List<Base> bases;

for (Base base : bases) {
  if (base instanceof Bar.class) {
     //execute my custom operation on the base object
     doSomething((Bar) base);
  }
}

If that approach is not that nice in general, how could I do better?

Upvotes: 6

Views: 1670

Answers (3)

Burkhard
Burkhard

Reputation: 14738

abstract class Base;//abstract function doSomething()
class Foo extends Base;//implements doSomething()
class Bar extends Base;//dito

List<Base> bases;

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

To answer your question: it is not a good idea to use instanceof.

Upvotes: 2

John Kane
John Kane

Reputation: 4443

There does not really seem to be any reason to use instance of here. It might make sense to have the base class default the behavior to doing nothing and override it in extending classes when needed. This way you only override it if needed (I on.y left this as an abstract class to follow with the question its not needed for this example). For example:

abstract class Base{
    public void doSomething(){}
}

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

public class B1 extends Base{}

An example of using this could be something like:

public class SomeOtherClass{
    public void something(List<Base> bases){
         for(Base base:bases)
             base.doSomething();
    }
}

Upvotes: 3

Marek
Marek

Reputation: 1878

Instance of is not a good practice here.

Correct solution will depend on what exactly is going on inside that doSomething method. If you do it your way then, besides other things, you violate Liskov Substitution Principle. I assume that you decided that you need these hierarchy in the first place because of something and I also assume that subtypes have some more behavior than only doSomething method. In this case, what you can do is shown below. Basically only types that should doSomething actually do it and rest of the types do something like no operation. In that way you can use these objects without needing to know what type they really are.

You should also ask yourself if you really need Base class to be abstract class. Maybe all you need is a interface. There might be better approach but basing on the information I have and what I've assumed then this seems to be alright.

public abstract class Base
{
    public abstract void doSomething();

    public void someOtherMethod()
    {
        // which does stuff
    }
}

public class SubTypeWhichCanDoSomething extends Base
{
    @Override
    public void doSomething()
    {
        // actually implement method and DO something
    }
}

public class DoesNothing extends Base
{
    @Override
    public void doSomething()
    {
        // does nothing
        return;
    }
}

// then your code looks like these
for(Base base : bases)
{
    base.doSomething();
}

Upvotes: 1

Related Questions