CODEWITHSUNDEEP
javaoopinstanceof
Jonas Bartkowski
Jonas Bartkowski

Reputation: 387

Am I using instanceof "wrong"?

So in a simple game engine of mine I use an interface "UpdatedGameElement" to signal that an object having that interface has to be updated every frame through an implementation of an update() method.

Now in my "main"(not THE main) class I iterate through a list of GameElement 's and check if these are an instanceof UpdatedGameElement. If this is the case I cast them, and then call .update().

Now the thing is, I just read that using instanceof is usually a sign of bad coding; and in cases where classes are used as markers when they could be easily replaced with a variable, I agree. But I'm not so sure about my case.

I guess I could let the GameElement class implement UpdatedGameElement, and define a standard empty update() method that needs to be overridden to actually do something, but I'm not sure if and why that would be better than what I have now.

What would you say?

Edit: some code from my main class:

public void process()
{
    if (active)
    {
        for (GameElement GE: elements)
        {
            if (!GE.isToBeRemoved())
            {
                //Relevant part
                if (GE instanceof UpdatedGameElement)
                {
                    ((UpdatedGameElement) GE).update();
                }           
            }
            else
            {
                prepareRemoval(GE);
            }
        }  
        processRemovals();
    }
}

Upvotes: 4

Views: 353

Answers (3)

laune
laune

Reputation: 31290

Following the invitation if the OP:

If use of the interface has no other reason than to add a marker plus the update method to GEs, and if the type UGE isn't used except after this single instanceof, then it is a weak reason for having these extra types. ESpecially when the capability of being updated can be extended to all other GEs, where it is just a NOOP.

An abstract method in the base class forces the programmer to decide whether update needs to be coded for a particular subclass. This approach is rather "safe" from a "defensive design" point of view. But, of course, you write more code.

In contrast to the previous technique: If you forget the interface there's no alarm.

Also, if you code a NOOP update method in the base class and rely on programmers' alacrity to override where necessary: convenient, but risky when you forget to do it.

Summarizing: There are a few subtle pro's and con's - not just the instanceof "smell".

Upvotes: 2

Nir Alfasi
Nir Alfasi

Reputation: 53525

I guess I could let the GameElement class implement UpdatedGameElement, and define a standard empty update() method that needs to be overridden to actually do something

Yes, you should definitely do it!

Reason: suppose in the future you'll need to implement another "updateable" class that extends UpdatedGameElement - consider the code changes you'll have to do in every place you've used instanceof...

Upvotes: 0

Programmer
Programmer

Reputation: 325

Based on your comments you mentioned that GameElement implements UpdatedGameElement, for now GameElement is the only class which is implementing UpdatedGameElement but it could be more in future. Also I hope you are aware that an interface cannot be instantiated thus you cannot create an instance of UpdatedGameElement thus in real the instances are created of the implementing classes of an interface. Thus during runtime when you create an instance of GameElement and assign it to UpdatedGameElement variable, that doesn't mean the instance is of type UpdatedGameElement now, its actually of type GameElement, now suppose that you have one more class implementing XYZElement implements UpdatedGameElement and create instance as below:

UpdatedGameElement ge = new GameElement();
UpdatedGameElement xyze = new XYZElement();

Do you think it good to use instance of check as below, as in either case it will be true and you never know which kind of instance ge and xyze are of.

if(ge instance of UpdatedGameElement)
instead one should always check for if(ge instance of GameElement)

Similarly for if(xyze instance of UpdatedGameElement)
instead one should always check for if(ge instance of XYZElement)

Hope this helps.

Upvotes: 0

Related Questions