During university lecture lecturer said that using getClass
and instanceof
indicates a bad design.
What are example usages that are bad design? What problems can be caused by using these methods? Are there any valid usages of these methods, which are not bad design?
I would say that it's a sign of bad design in most cases. For example, let's say that you have a list of objects and you are doing instanceof
, followed by the cast, followed by calling a method specific for the class. Instead, these objects should have common superclass and the method should be declared there - then different code will be executed depending on the actual type of object (since subclass may define different implementation).
private static class A {
private void printA() {
System.out.println("A");
}
}
private static class B {
private void printB() {
System.out.println("B");
}
}
public static void main(String[] args) {
List<Object> list = asList(new A(), new B(), new A());
list.forEach(element -> { // this is bad, don't do it!
if (element instanceof A) {
((A) element).printA();
}
if (element instanceof B) {
((B) element).printB();
}
});
}
Instead, you should do this:
private interface Printer {
void print();
}
private static class A implements Printer {
@Override
public void print() {
System.out.println("A");
}
}
private static class B implements Printer {
@Override
public void print() {
System.out.println("B");
}
}
public static void main(String[] args) {
List<Printer> list = asList(new A(), new B(), new A());
list.forEach(Printer::print);
}
A valid use case you will see in autogenerated equals
methods. Before actually comparing objects, there is a check whether they are of the same class. If they are not, they cannot be equal so there is a fail-fast optimisation. This is actually enforced by equals
method taking a parameter of type Object
. Even if two objects we are comparing are actually equal, we will have to cast the parameter and before doing this we should check its class in order to return false
rather then getting ClassCastException
.
Equals method generated by IntelliJ:
public class Person {
private String name;
private String surname;
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Person person = (Person) o;
if (!name.equals(person.name)) return false;
return surname.equals(person.surname);
}
}
Another valid case of using these methods is for example when creating various tools such as POJO to json mappers, which can be done only via reflection API.
EDIT:
Following your questions in comments, here is the working example of how to implement a list of animals, where dog can run and eagle can both run and fly:
public static abstract class Animal {
protected final String name;
public Animal(String name) {
this.name = name;
}
public void run() {
System.out.println(name + " runs");
}
public abstract void move();
}
public static class Dog extends Animal {
public Dog() {
super("Dog");
}
@Override
public void move() {
run();
}
}
public static class Eagle extends Animal {
public Eagle() {
super("Eagle");
}
public void fly() {
System.out.println(name + " flies");
}
@Override
public void move() {
fly();
}
}
public static void main(String[] args) {
List<Animal> animals = Arrays.asList(new Dog(), new Eagle());
animals.forEach(Animal::move);
System.out.println("Eagle can run too!");
new Eagle().run();
}
Output:
Dog runs
Eagle flies
Eagle can run too!
Eagle runs
It is all about analysing how the code is used and extracting common parts. If in the loop you were always ordering animal to run, then the cast would not be needed since run()
is declared on Animal
. Here on the other hand we want animal to move, doesn't matter how, so let them choose their default movement type by creating abstract move()
method in Animal
class.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With