Рефакторинг абстрактного Java-класса со многими дочерними классами - PullRequest
1 голос
/ 22 сентября 2009

Я ищу идеи о лучшем способе рефакторинга этого сценария (лучший дизайн, минимальные усилия). Начиная со следующего примера абстрактного класса (в фактическом есть еще много полей, методов и абстрактных методов):

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();
}

Существует так много дочерних классов (скажем, 100), которые расширяют этот класс. Эти детские классы считаются «спецификациями» для автомобилей. Вот два примера:

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;
    }
}

Во время выполнения создаются и используются объекты автомобилей:

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

Однако, получив некоторые внешние данные, я обнаружил, что автомобиль был перекрашен, а двигатель заменен. Новые спецификации для автомобиля стали:

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;
    }
}

Так что действительно нужно «применить» эти спецификации к новому carObject без изменения существующих полей, таких как manufactureDate. Проблема состоит в том, как минимизировать код изменений этих 100+ дочерних классов (желательно, чтобы они оставались нетронутыми), при этом имея возможность обновлять carObject во время выполнения.

N.B. Мне дали работать над этим кодом, поэтому я не писал его в таком состоянии с самого начала.

Ответы [ 4 ]

8 голосов
/ 22 сентября 2009

На основании описания и примера вы используете наследование ненадлежащим образом. Похоже, вы создаете много классов, где вы должны использовать один класс и множество экземпляров объекта. Если это правда, вам также не нужен шаблон проектирования для решения проблемы. Без дальнейшего выяснения проблемы этого должно быть достаточно:

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; }
}

Пример использования:

// 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);

Если вы хотите минимизировать изменения, вы можете использовать мой класс Car с рефакторингом сверху, а затем очистить дочерние классы, чтобы они использовали его. Что-то вроде:

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
}

Так как на самом деле существует базовый класс, я предполагаю, что 99% кода не ссылаются на конкретные классы автомобилей (только базовый класс), поэтому вы должны иметь возможность довольно легко что-то менять. Конечно, трудно сказать, не видя реального кода.

1 голос
/ 22 сентября 2009

Это зависит от того, насколько вы контролируете код, который создает эти объекты. Я собираюсь предположить, что этот дизайн существует по причине, которая была отчасти потеряна в примере с автомобилем, но если объекты создаются путем вызова new, то мало что можно сделать, кроме как изменить их, хотя вы могли бы использовать Остальная часть этого ответа предлагает более гибкий способ их изменения.

Если вы можете контролировать их создание, то фабрика, которая использует композицию и возвращает другой тип автомобильного объекта, который переопределяет конкретные параметры, которые вас интересуют, и вызывает оригинал для остальных, позволит вам повлиять на ваши изменения в конкретном экземпляре. без изменения всех оригинальных классов. Что-то вроде:

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

Затем внутри этого метода makeCar вы можете решить, следует ли возвращать объект CarOne или составную реализацию:

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; }
}
0 голосов
/ 22 сентября 2009

Ваши подклассы не обеспечивают различного поведения только различного данных .

Следовательно, вы не должны использовать разные подклассы, только разные аргументы.

Я бы предложил добавить метод getCar в базовый вариант и использовать его как заводской метод.

Добавьте свойства «Цвет» и «Цилиндр» и загрузите их из ... в любое удобное для вас место, это может быть база данных, файл свойств, фиктивный объект, из Интернета, из космического пространства ... и т. Д.

До:

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

После того, как:

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; }


}

Таким образом, вместо того, чтобы создавать новый автомобиль напрямую, вы получите его с помощью метода getCar:

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

Я бы не советовал вам делать ваш автомобиль "изменчивым", так как он может вызвать незначительные нежелательные побочные эффекты (поэтому я отмечаю атрибуты как окончательные). Поэтому, если вам нужно «модифицировать» свой автомобиль, вам лучше назначить новые атрибуты:

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

Кроме того, вы даже можете нанести на карту всю машину, чтобы использовать только один экземпляр.

При этом вам не нужно добавлять сеттеры в ваш код. Единственное, что вам нужно сделать, это найти и заменить

  Car xyz = new WhatEver( number );

Для

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

А остальная часть кода должна работать без изменений.

0 голосов
/ 22 сентября 2009

Я бы также рекомендовал взглянуть на Шаблон строителя , если у вас есть кейсы (или целые группы классов), которые имеют сложную логику построения, особенно если некоторые поля требуются в некоторых автомобилях, и разные наборы полей обязательны в других.

...