Creating many object types/classes just to use instanceof

Question

I'm wondering if I'm creating/breaking down object types into meaningless classes and if there is a better way to do it.

Say I've got the following classes (--> extends the higher class):

Vehicle class - takes care of movement and a default speed
-->Car class - adds passengers
-->-->ElectricCar class - a default constructor
-->-->GasolineCar class - a default constructor
-->-->HybridCar class - a default constructor
-->Motorcycle class - a default constructor
-->Bike class - a default constructor, overrides speed
-->Segway class - a default constructor, overrides speed

As you can see, they're mostly default constructors. So I'm only creating them in case I ever need to use an instanceof conditional statement. Most of my code ends up in the Vehicle class because I'm trying to avoid repeating the same code in every class. Also, I should make Vehicle abstract, correct? I never create a Vehicle object.

Is this OK or is there a better way to do it?

I am interested in a way you are going to use these classes. — SMA, Mar 22 '15 at 14:21
it all depends how you are going to use them. Nothing wrong with your structure so far. And you may not only use it for instanceof but for example for methods like boollean needsPetrol()... canHavePassengers() which you will implement in the subclasses instead of using chain of if instanceof statements. — Zielu, Mar 22 '15 at 14:22
Just keep one thingy in mind, avoid the use of `instanceof` inside the code. It is like asking the third party to tell the structure of your classes, in this it is askiing the compiler. Instead create a function like `public boolean isMotercycle() {return true;}` inside motorcycle, though in vehicle class, use `public boolean isMotercycle () {return false;}`. This is like the class itself giving information about itself — nIcE cOw, Mar 22 '15 at 14:24
I'm programming a game, and these would all be different game objects. When I'm doing collision detection, different stuff happens based on what type of object was involved in the collision. These aren't the exact object types, but it's a similar example. Ah, thanks for that nIcE cOw, that makes sense. — SmokeyTehBear, Mar 22 '15 at 14:27
See also http://stackoverflow.com/questions/23367491/whats-the-best-way-to-handle-type-indicators-in-object-oriented-design — Raedwald, Mar 23 '15 at 07:36
Possible duplicate of http://stackoverflow.com/questions/5579309/switch-instanceof — Raedwald, Mar 23 '15 at 07:37

Elliott Frisch · Answer 1 · 2015-03-22T14:26:56.990

4

First, as long as the code functions and solves the task at hand I would say it is "correct" enough (so yes, it is okay). That being said, some people prefer to use interface(s) instead of abstract classes (because you can implement multiple interfaces but only directly extend from one ancestor). Further, if you're using Java 8+ you can add default methods to interface(s). Other alternatives might include having one Vehicle class but a VehicleType enum field for the Vehicle class.

edited Mar 22 '15 at 14:26

answered Mar 22 '15 at 14:21

Elliott Frisch

198,278
20
158
249

1

@Downvoter: Atleast share your thoughts too, while downvoting. It helps the community in moving forward, by knowing their mistakes – nIcE cOw Mar 22 '15 at 14:26
1

@nIcEcOw I *always* appreciate constructive feedback and the opportunity to improve my answers. – Elliott Frisch Mar 22 '15 at 14:28
Thanks for the advice, I'll think about the an enum/state as an alternative. Only one more point and I can upvote some answers lol. – SmokeyTehBear Mar 22 '15 at 14:41

score 4 · Answer 2 · answered Mar 22 '15 at 14:30

There's never such a thing as "too many classes" as long as your classes make sense and serve a function. There is however such a thing as pointless abstraction in my opinion.

If you're 100% sure that you will only ever use these classes to determine the vehicle type (quote from your question),

I ever need to use an instanceof conditional statement

Then maybe having sub classes qualifies as pointless abstraction. Just use an enum instead which describes your vehicles type and move on.

If however you anticipate or plan on using your sub classes for more than just the type, it's completely fine to do it by having sub classes and it's in fact preferred to having lots of if/else or switch statements in your Vehicle class.

Ok cool, so as things are right now, an enum may be preferable. Thank you. — SmokeyTehBear, Mar 22 '15 at 14:43

score 4 · Answer 3 · answered Mar 22 '15 at 15:12

As already pointed out in the comments and answers: It all depends on what you are trying to model there.

But first of all, regarding the intention that you mentioned:

So I'm only creating them in case I ever need to use an instanceof conditional statement.

Be careful with that. You should avoid modeling the different behavior of objects depending on their type. Here, it does not matter whether the type information is

implicitly available via instanceof checks
explicitly via a method like boolean isMotorCycle(), as suggested in a comment
explicitly via something like an enum Type { ... } and a switch over the type.

They all suffer from the same problem: When you add a new class/type to your hierarchy, then you have to adopt and update all places where this type information was queried. This may become a maintainance nightmare.

There are legitimate uses for type checks. But when you end up with writing code that frequently does checks like this

void recharge(Vehicle v) {
    if (v instanceof ElectricCar) ((ElectricCar)v).attachPlug();
    if (v instanceof GasolineCar) ((GasolineCar)v).fillFuel();
    if (v instanceof Bike)        ((Bike)v).getDriver().takeRest();
    ...
}

then this is a strong indication that something is wrong with your hierarchy. It might be that the base class, Vehicle, is simply not powerful enough. In the example above, one could then consider pulling the recharge() method into the Vehicle class, and simply call it, relying on the polymorphic implementation.

It might, in the worst case, also be that the concepts that you are trying to model are simply too unrelated to be combined in a single hierarchy.

You mentioned that you want to do collision detection for these objects. This sounds like you're already approaching a solution like this:

class Bike extends Vehicle {
    @Override 
    void collideWith(Vehicle other) {
        if (other instanceof Car) collideWithCar((Car)other);
        if (other instanceof Segway) collideWithSegway((Segway)other);
        ...
    }
}

There are more elegant solutions for that. You might want to read about the problem related to Double Dispatch, and maybe have a look at the Strategy Pattern.

More generally, concerning the question of whether you have "too many classes", there are some general rules fo thumb.

One of them is the Interface Segration Principle, stating that you should create serval client-specific insterfaces, instead of one large interface that does everything and therefore combines otherwise unrelated methods. So you don't have "too many classes", as long as each of them serves a purpose. You could ask yourself:

Who will have to differentiate between a GasolineCar and an ElectricCar?
Who will have to deal with only one of these interfaces?
When they both extend the same base class: Who will be able to use the base class (or will everybody have to know the specific implementation anyhow?)

Thank you for the in depth answer. Your recharge() example makes perfect sense and does eliminate the conditional statements I was using with both the **instanceof** and isMotorcyle() approaches. And that's what I couldn't wrap my head around, trying to do it in the most polymorphic way. It's hard for me to figure out who recommended the best approach because there are so many ways to go about it, but all parts of your answer seem spot on. — SmokeyTehBear, Mar 22 '15 at 15:38

score 2 · Answer 4 · answered Mar 22 '15 at 14:24

It all comes down to question if you will ever need to distinguish between ElectricCars, GasolineCars etc. If not, you could just put everything into Vehicle or Car and create Vehicle/Car objects.

If it is not intended to create Vehicle objects, then yes, it should be marked abstract.

Creating many object types/classes just to use instanceof

4 Answers4