Refactoring abstract Java class with many child classes

Question

I'm looking for ideas on the best way to refactor this scenario (better design, minimal effort). Starting from the following example abstract class (actual has many more fields, methods and abstract methods) :

abstract class Car
{
    private int manufactureYear;
    // ... many more fields that are hard to clone

    public Car(int manYear)
    {
      this.manufactureYear = manYear;
    }

    abstract public Color getColor();
    abstract public int getNumCylinders();
}

There are so many child classes (say 100) that extend this class. These child classes are considered like 'specs' for the cars. Here are two examples :

class CarOne extends Car
{
    private static Color COLOR = Color.Red;
    private static int CYLINDERS = 4;

    public CarOne(int manYear)
    {
      super(manYear);
    }

    public final Color getColor();
    {
    return COLOR;
    }

    public final int getNumCylinders() 
    {
    return CYLINDERS;
    }
}

class CarOneThousand extends Car
{
    private static Color COLOR = Color.Black;
    private static int CYLINDERS = 6;

    public CarOneThousand(int manYear)
    {
      super(manYear);
    }

    public final Color getColor();
    {
        return COLOR;
    }

    public final int getNumCylinders() 
    {
    return CYLINDERS;
    }
}

During runtime car objects get instantiated and used:

CarOne carObject = new CarOne(2009);
carObject.getColor();
carObject.getNumCylinders();

However, after getting some external data, I discover that the car was repainted and the engine changed. The new specs for the car become:

class ModCar extends Car
{
    private static Color COLOR = Color.Blue; 
    private static int numCylinders = 8;

    public ModCar (int manYear)
    {
      super(manYear);
    }

    public final Color getColor();
    {
    return COLOR;
    }

    public final int getNumCylinders() 
    {
    return numCylinders;
    }
}

So really need to "apply" these specs to the new carObject without modifying existing fields such as manufactureDate. The problem is how to minimize the code of changes to those 100+ child classes (preferably leave them untouched) while being able to update the carObject during runtime.

N.B. I was given to work on this code so I didn't write it in this condition to begin with.

Answer 1

Based on the description and example, you are using inheritance inappropriately. It looks like you are creating many classes where you should be using a single class and many object instances. If this is true, you also don't need a design pattern to solve the problem. Without further clarification of the problem, this should suffice:

class Car
{
    private int manufactureYear;
    private Color color;
    private int numCylinders;

    public int getManufactureYear() { return manufactureYear; }
    public void setManufactureYear(int manufactureYear) { this.manufactureYear = manufactureYear; }

    public Color getColor() { return color; }
    public void setColor(Color color) { this.color = color; }

    public int getNumCylinders() { return numCylinders; }
    public void setNumCylinders(int numCylinders) { this.numCylinders = numCylinders; }
}

Example usages:

// make a blue 6-cylinder:
Car blue6 = new Car();
blue6.setColor(BLUE);
blue6.setCylinders(6);

// make a red 4-cylinder:
Car red4 = new Car();
red4.setColor(RED);
red4.setCylinders(4);

// Uh-oh, they painted my red car!
red4.setColor(YELLOW);

If you want to minimize changes, you could use my refactored Car class from above, and then clean up the child classes so they leverage it. Something like:

class CarOne extends Car { // extends my version of Car...

    private static Color COLOR = Color.Red;
    private static int CYLINDERS = 4;

    public CarOne() {
      setColor(COLOR);
      setNumCylinders(CYLINDERS );
    }

    // getters deleted, base class has them now
}

Since there is in fact a base class, my guess is that 99% of the code does not reference the concrete car classes (only the base class), so you should be able to change things fairly easily. Of course, hard to say without seeing the real code.

Answer 2

It depends on how much control you have over the code that creates these objects. I'm going to assume that this design exists for a reason that was kind of lost in the car example, but if the objects are created by calling new, then there is little you can do other than change them, although you could use the rest of this answer to suggest a more flexible way to change them.

If you can control their creation, then a factory that uses composition and returns a different kind of car object that overrides the specific parameters you care about and calls the original for the rest would allow you to affect your changes on a specific instance without changing all of the original classes. Something like:

Car carOne = CarFactory.makeCar("CarOne", 2009);

Then inside that makeCar method, you can decide whether or not to return a CarOne object, or a composite implementation:

public class CompositeCar extends Car {


      private Car original;
      private Color myColor;

      public CompositeCar(Car original, Color myColor) {
          this.original = original;
          this.myColor = myColor;
      }

      public int getYear() { return original.getYear(); }

      public Color getColor() { return myColor; }
}

Answer 3

I'd also recommend taking a look at the Builder Pattern if you have cases (or entire groups of classes) which have complicated construction logic, especially if some fields are required in some Cars, and different sets of fields are required in others.

Answer 4

Your subclasses do not provide different behavior only different data.

Hence you should not use different subclasses only different arguments.

I would suggest add a "getCar" method to your base case and use it as an a factory method.

Add the Color and Cylinder properties and load them from ... anywhere it suits your needs, it may be a database, a properties file, a mock object, from the internet, from a cosmic place ... etc.

Before:

Car car = new CarOne(2009); // Using new to get different data....
carObject.getColor();
carObject.getNumCylinders();

After:

class Car {
    // Attributes added and marked as final.
    private final Color color;
    private final int numberCylinders;
    // original 
    private final int manufacteredYear;

    public static Car getCar( String spec, int year ) {

          return new Car( year, 
                          getColorFor( spec ) , 
                          getCylindersFor(spec) );

    }

    // Make this private so only the static method do create cars. 
    private Car( int year, Color color, int cylinders ) {
         this.manufacturedYear = year;
         this.color = color;
         this.numberCylinders = cylinders;
    }

    // Utility methods to get values for the car spec.
    private static final getColorFor( String spec ) {
       // fill either from db, xml, textfile, propertie, resource bundle, or hardcode here!!!
       return ....
    }
    private static final getCylindersFor( String spec ) {
       // fill either from db, xml, textfile, propertie, resource bundle, or hardcode here!!!
       return .... 
    }

    // gettes remain the same, only they are not abstract anymore.
    public Color getColor(){ return this.color; }
    public int getNumCylinders(){ return this.numberCylinders; }


}

So instead of create a new car directly you would get it from the getCar method:

Car car = Car.getCar("CarOne", 2009 );
....

I wouldn't recommend you to make your car "mutable" for it may bring subtle undesired side effects ( that's why I mark the attributes as final ) . So if you need to "modify" you car, you better assign new attributes:

 Car myCar = Car.getCar("XYZ", 2009 );
 .... do something with car
 myCar = Car.getCar("Modified", 2009 );
 //-- engine and color are "modified"

Additionally you may even map the whole car so you only use one instance.

By doing this you don't have to add setters to your code. The only thing you have to do would be search and replace

  Car xyz = new WhatEver( number );

For

 Car xyz = Car.getCar("WhatEver", number );

And the rest of the code should run without changes.