Geek Answers Handbook

Uses .getClass () considered bad design?

I am currently implementing a function using a superclass as a parameter.

For instance:

private void foo(Parent parent) { if(parent.getClass() == Child1.class) { Child1 c1 = (Child1) parent; System.out.println(c1.getChild1Attribute()); } else if(parent.getClass() == Child2.class) { Child2 c2 = (Child2) parent; System.out.println(c1.getChild2Attribute()); } else if(parent.getClass() == Parent.class) { System.out.println(parent.getParentAttribute()); } }

It is a bad idea?

I read some topics here saying using getClass() or instanceof is a bad design:

+4
source share
5 answers

This is not necessarily a bad design, but it is a sign that something is wrong.

Your particular case looks bad, because it seems that one method knows about several classes. This may indicate that you do not have the ability to overload or the ability to use one of several sending templates:

 // Three overloads - one per target class private void foo(Parent obj) { } private void foo(Child1 obj) { } private void foo(Child2 obj) { }

One of the common mailing templates is the visitor template , see if it applies to the problem you are solving.

+7
source

Yes, this is a bad design.

Instead, you should make one abstract method in the superclass and override it in each subclass to perform the desired action.

+6
source

Yes, this is a sign of poor design. This puts the complexity of processing different classes in one class instead of encapsulating the corresponding knowledge in the corresponding classes. It will be painful if you add more classes to your hierarchy, as the compiler will not remind you to implement the corresponding new functions for foo .

The best version would be

 private void foo(Parent parent){ System.out.println(parent.getFooParentAttribute()); }

Then we implement getFooParentAttribute for each of the classes.

+2
source

Prefer instanceof approach

Josh Bloch by design

The reason I support the instanceof method is that when you use the getClass approach, you have a restriction on the fact that objects are equal only to other objects of the same class, to the same type of runtime.

+1
source

There are two problems with your as-is method. First, suppose you add a new subclass of the Parent class, Child3 . This does not apply to any of your cases, so it does not print anything. The same is true if you add a new subclass of Child1 , ChildOfChild1 . It will also not be covered.

If you used instanceof instead, then ChildOfChild1 will appear as an instance of Child1 , and Child3 will be an instance of Parent .

But, as a rule, the reason you want to completely avoid this pattern is that all these cases may be unexpected. What is generally better to write

 void foo(Parent p) {...}

and put the general code there, and then for any special cases, create

 void foo(Child1 c) {...}

This makes it clearer what happens: if you see these two methods, you know that Child1 (and its subclasses) has special code, and Parent and all other subclasses are handled in the same way.

0
source

More articles:


All Articles