I am attempting to implement my first Factory Design Pattern, and I'm not sure how to avoid using instanceof when adding the factory-made objects to lists. This is what I'm trying to do:
for (Blueprint bp : blueprints) { Vehicle v = VehicleFactory.buildVehicle(bp); allVehicles.add(v); // Can I accomplish this without using 'instanceof'? if (v instanceof Car) { cars.add((Car) v); } else if (v instanceof Boat) { boats.add((Boat) v); } else if (v instanceof Plane) { planes.add((Plane) v); } }
From what I've read on Stack Overflow, using 'instanceof' is a code smell. Is there a better way to check the type of vehicle that was created by the factory without using 'instanceof'?
I welcome any feedback/suggestions on my implementation as I'm not even sure if I'm going about this the right way.
Full example below:
import java.util.ArrayList; class VehicleManager { public static void main(String[] args) { ArrayList<Blueprint> blueprints = new ArrayList<Blueprint>(); ArrayList<Vehicle> allVehicles = new ArrayList<Vehicle>(); ArrayList<Car> cars = new ArrayList<Car>(); ArrayList<Boat> boats = new ArrayList<Boat>(); ArrayList<Plane> planes = new ArrayList<Plane>(); /* * In my application I have to access the blueprints through an API * b/c they have already been created and stored in a data file. * I'm creating them here just for example. */ Blueprint bp0 = new Blueprint(0); Blueprint bp1 = new Blueprint(1); Blueprint bp2 = new Blueprint(2); blueprints.add(bp0); blueprints.add(bp1); blueprints.add(bp2); for (Blueprint bp : blueprints) { Vehicle v = VehicleFactory.buildVehicle(bp); allVehicles.add(v); // Can I accomplish this without using 'instanceof'? if (v instanceof Car) { cars.add((Car) v); } else if (v instanceof Boat) { boats.add((Boat) v); } else if (v instanceof Plane) { planes.add((Plane) v); } } System.out.println("All Vehicles:"); for (Vehicle v : allVehicles) { System.out.println("Vehicle: " + v + ", maxSpeed: " + v.maxSpeed); } System.out.println("Cars:"); for (Car c : cars) { System.out.println("Car: " + c + ", numCylinders: " + c.numCylinders); } System.out.println("Boats:"); for (Boat b : boats) { System.out.println("Boat: " + b + ", numRudders: " + b.numRudders); } System.out.println("Planes:"); for (Plane p : planes) { System.out.println("Plane: " + p + ", numPropellers: " + p.numPropellers); } } } class Vehicle { double maxSpeed; Vehicle(double maxSpeed) { this.maxSpeed = maxSpeed; } } class Car extends Vehicle { int numCylinders; Car(double maxSpeed, int numCylinders) { super(maxSpeed); this.numCylinders = numCylinders; } } class Boat extends Vehicle { int numRudders; Boat(double maxSpeed, int numRudders) { super(maxSpeed); this.numRudders = numRudders; } } class Plane extends Vehicle { int numPropellers; Plane(double maxSpeed, int numPropellers) { super(maxSpeed); this.numPropellers = numPropellers; } } class VehicleFactory { public static Vehicle buildVehicle(Blueprint blueprint) { switch (blueprint.type) { case 0: return new Car(100.0, 4); case 1: return new Boat(65.0, 1); case 2: return new Plane(600.0, 2); default: return new Vehicle(0.0); } } } class Blueprint { int type; // 0 = car; // 1 = boat; // 2 = plane; Blueprint(int type) { this.type = type; } }
The primary alternative to using instanceof is polymorphism. Rather then ask which type of object you have at the current position you tell the object, whatever it is, to do what you want done. If both objects know how to do that then this works fine, even if they do it differently.
Having a chain of "instanceof" operations is considered a "code smell". The standard answer is "use polymorphism".
Yes, in general using `instanceof`` is a symptom of a bad/poor design. The trick is to use double dispatch or visitor pattern. Look on the web. It would be easier to give you some examples if you included a bit of detail about how you are using instanceof .
You could implement the Visitor pattern.
Detailed Answer
The idea is to use polymorphism to perform the type-checking. Each subclass overrides the accept(Visitor)
method, which should be declared in the superclass. When we have a situation like:
void add(Vehicle vehicle) { //what type is vehicle?? }
We can pass an object into a method declared in Vehicle
. If vehicle
is of type Car
, and class Car
overrode the method we passed the object into, that object would now be processed within the method declared in the Car
class. We use this to our advantage: creating a Visitor
object and pass it to an overriden method:
abstract class Vehicle { public abstract void accept(AddToListVisitor visitor); } class Car extends Vehicle { public void accept(AddToListVisitor visitor) { //gets handled in this class } }
This Visitor
should be prepared to visit type Car
. Any type that you want to avoid using instanceof
to find the actual type of must be specified in the Visitor
.
class AddToListVisitor { public void visit(Car car) { //now we know the type! do something... } public void visit(Plane plane) { //now we know the type! do something... } }
Here's where the type checking happens!
When the Car
receives the visitor, it should pass itself in using the this
keyword. Since we are in class Car
, the method visit(Car)
will be invoked. Inside of our visitor, we can perform the action we want, now that we know the type of the object.
So, from the top:
You create a Visitor
, which performs the actions you want. A visitor should consist of a visit
method for each type of object you want to perform an action on. In this case, we are creating a visitor for vehicles:
interface VehicleVisitor { void visit(Car car); void visit(Plane plane); void visit(Boat boat); }
The action we want to perform is adding the vehicle to something. We would create an AddTransportVisitor
; a visitor that manages adding transportations:
class AddTransportVisitor implements VehicleVisitor { public void visit(Car car) { //add to car list } public void visit(Plane plane) { //add to plane list } public void visit(Boat boat) { //add to boat list } }
Every vehicle should be able to accept vehicle visitors:
abstract class Vehicle { public abstract void accept(VehicleVisitor visitor); }
When a visitor is passed to a vehicle, the vehicle should invoke it's visit
method, passing itself into the arguments:
class Car extends Vehicle { public void accept(VehicleVisitor visitor) { visitor.visit(this); } } class Boat extends Vehicle { public void accept(VehicleVisitor visitor) { visitor.visit(this); } } class Plane extends Vehicle { public void accept(VehicleVisitor visitor) { visitor.visit(this); } }
That's where the type-checking happens. The correct visit
method is called, which contains the correct code to execute based on the method's parameters.
The last problem is having the VehicleVisitor
interact with the lists. This is where your VehicleManager
comes in: it encapsulates the lists, allowing you to add vehicles through a VehicleManager#add(Vehicle)
method.
When we create the visitor, we can pass the manager to it (possibly through it's constructor), so we can perform the action we want, now that we know the object's type. The VehicleManager
should contain the visitor and intercept VehicleManager#add(Vehicle)
calls:
class VehicleManager { private List<Car> carList = new ArrayList<>(); private List<Boat> boatList = new ArrayList<>(); private List<Plane> planeList = new ArrayList<>(); private AddTransportVisitor addVisitor = new AddTransportVisitor(this); public void add(Vehicle vehicle) { vehicle.accept(addVisitor); } public List<Car> getCarList() { return carList; } public List<Boat> getBoatList() { return boatList; } public List<Plane> getPlaneList() { return planeList; } }
We can now write implementations for the AddTransportVisitor#visit
methods:
class AddTransportVisitor implements VehicleVisitor { private VehicleManager manager; public AddTransportVisitor(VehicleManager manager) { this.manager = manager; } public void visit(Car car) { manager.getCarList().add(car); } public void visit(Plane plane) { manager.getPlaneList().add(plane); } public void visit(Boat boat) { manager.getBoatList().add(boat); } }
I highly suggest removing the getter methods and declaring overloaded add
methods for each type of vehicle. This will reduce overhead from "visiting" when it's not needed, for example, manager.add(new Car())
:
class VehicleManager { private List<Car> carList = new ArrayList<>(); private List<Boat> boatList = new ArrayList<>(); private List<Plane> planeList = new ArrayList<>(); private AddTransportVisitor addVisitor = new AddTransportVisitor(this); public void add(Vehicle vehicle) { vehicle.accept(addVisitor); } public void add(Car car) { carList.add(car); } public void add(Boat boat) { boatList.add(boat); } public void add(Plane plane) { planeList.add(plane); } public void printAllVehicles() { //loop through vehicles, print } } class AddTransportVisitor implements VehicleVisitor { private VehicleManager manager; public AddTransportVisitor(VehicleManager manager) { this.manager = manager; } public void visit(Car car) { manager.add(car); } public void visit(Plane plane) { manager.add(plane); } public void visit(Boat boat) { manager.add(boat); } } public class Main { public static void main(String[] args) { Vehicle[] vehicles = { new Plane(), new Car(), new Car(), new Car(), new Boat(), new Boat() }; VehicleManager manager = new VehicleManager(); for(Vehicle vehicle : vehicles) { manager.add(vehicle); } manager.printAllVehicles(); } }
You can add method to vehicle class to print the text. Then override the method in each specialized Car class. Then just add all the cars to the vehicle list. And loop the list to print the text.
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