Композиция по наследству . Кажется, что мы все согласны и понимаем этот ценный принцип. В последнее время я читал большое количество кода из некоторых проектов с открытым исходным кодом (включая мой), и я думаю, что нам все еще не хватает хотя бы одного варианта использования, чтобы применить его.
Это то, что я видел: у класса A есть метод x . Сразу после того, как метод x завершил свою работу и перед выходом, он вызывает (иногда пустой) защищенный метод y, предназначенный для переопределения, чтобы обработать некоторый результат или побочный эффект от x (есть имя для этого анти-паттерна?)
Вот конкретный (и упрощенный) пример, взятый из FEST :
/**
* Understands a <code>{@link SecurityManager}</code> that does not allow an application
* under test to terminate the current JVM. Adapted from Abbot's own
* {@code SecurityManager}.
*/
public class NoExitSecurityManager extends SecurityManager {
@Override public final void checkExit(int status) {
if (!exitInvoked()) return;
handleExitRequest(status);
throw new ExitException(concat(
"Application tried to terminate current JVM with status ", status));
}
/**
* Implement this method to do any context-specific cleanup. This hook is provided since
* it may not always be possible to catch the <code>{@link ExitException}</code>
* explicitly (like when it's caught by someone else, or thrown from the event dispatch
* thread).
* @param status the status the exit status.
*/
protected void handleExitRequest(int status) {}
}
На первый взгляд этот код выглядит нормально. Подклассы могут делать все, что хотят, когда вызывается «exit», и, поскольку метод checkExit является окончательным, подклассы не могут случайно (или намеренно) изменить предполагаемое поведение суперкласса.
ИМХО есть некоторые проблемы с этим подходом:
- Подклассы могут потенциально нарушать принцип единой ответственности
- Ненужное наследство
- Сложное тестирование
Давайте посмотрим на простой (глупый?) Подкласс NoExitSecurityManager, который записывает сообщение в файл при вызове exit:
public class WriteToFileNoExitSecurityManager extends NoExitSecurityManager {
// The class FileWriter only exists for the purposes of this example
private final FileWriter fileWriter = new FileWriter("${temp}/log.txt");
@Override protected void handleExitRequest(int status) {
fileWriter.write(concat("Exit called with status ", status));
}
}
Этот подкласс на самом деле делает слишком много: он предотвращает выход приложения (унаследованное поведение), а также обрабатывает «запрос на выход».
WriteToFileNoExitSecurityManager на самом деле не NoExitSecurityManager, а «обработчик запроса на выход». Наследование в этом случае, IMHO, не нужно: реализованная функциональность (не унаследованная) в WriteToFileNoExitSecurityManager является автономной и не зависит от внутреннего состояния (или идентичности) суперкласса. Кроме того, поскольку метод checkExit является окончательным, может быть только одна реализация. С другой стороны, могут быть разные реализации handleExitRequest, каждая из которых должна быть подклассом NoExitSecurityManager.
Тестирование также сложнее в этом сценарии. Чтобы протестировать NoExitSecurityManager, нам нужно было бы разделить его на подклассы (более ненужное наследование) и установить некоторый флаг в handleExitRequest, чтобы убедиться, что он вызывается (мы также можем создать макет, но в конце он все тот же):
public class NoExitSecurityManagerTest {
private TestNoExitSecurityManager securityManager;
@Before public void setUp() {
securityManager = new TestNoExitSecurityManager();
}
@Test public void should_prevent_application_from_exiting_and_handle_exit_request() {
// test that an application trying to exit is effectively stopped
...
// here we verify that the call to 'handleExitRequest' was made
assertThat(securityManager.handleExitRequestCalled).isTrue();
}
private static class TestNoExitSecurityManager extends NoExitSecurityManager {
boolean handleExitRequestCalled;
@Override protected void handleExitRequest(int status) {
handleExitRequestCalled = true;
}
}
}
Введите шаблон стратегии
Мы можем очистить наш пример, используя шаблон стратегии .
Сначала мы представляем интерфейс ExitRequestHandler. У него только одна ответственность: обрабатывать «запросы на выход».
public interface ExitRequestHandler {
void handleExitRequest(int status);
}
Затем мы можем заменить вызов метода handleExitRequest вызовом экземпляра ExitRequestHandler:
/**
* Understands a <code>{@link SecurityManager}</code> that does not allow an application
* under test to terminate the current JVM. Adapted from Abbot's own
* {@code SecurityManager}.
*/
public final class NoExitSecurityManager extends SecurityManager {
private final ExitRequestHandler exitRequestHandler;
public NoExitSecurityManager(ExitRequestHandler exitRequestHandler) {
this.exitRequestHandler = exitRequestHandler;
}
@Override public void checkExit(int status) {
if (!exitInvoked()) return;
exitRequestHandler.handleExitRequest(status);
throw new ExitException(concat(
"Application tried to terminate current JVM with status ", status));
}
}
Наконец, мы можем переписать WriteToFileNoExitSecurityManager как WriteToFileExitRequestHandler:
public class WriteToFileExitRequestHandler implements ExitRequestHandler {
// The class FileWriter only exists for the purposes of this example
private final FileWriter fileWriter = new FileWriter("${temp}/log.txt");
@Override public void handleExitRequest(int status) {
fileWriter.write(concat("Exit called with status ", status));
}
}
Похоже, я просто переместил код и фактически создал еще больше кода! Правда, у нас больше кода, но кода лучшего качества:
- Каждый класс имеет одну четко определенную ответственность
- нет тесной связи между классом, использующим интерфейс стратегии и реализацию конкретной стратегии
- реализовать небольшой интерфейс проще, чем расширять класс (не нужно беспокоиться о состоянии суперкласса в определенном контексте)
- тестирование проще
Вот обновленный тест:
public class NoExitSecurityManagerTest {
private ExitSecurityManager securityManager;
private TestExitRequestHandler requestHandler;
@Before public void setUp() {
requestHandler = new ExitRequestHandler();
securityManager = new ExitSecurityManager(requestHandler);
}
@Test public void should_prevent_application_from_exiting_and_handle_exit_request() {
// test that an application trying to exit is effectively stopped
...
// here we verify that the call to 'handleExitRequest' was made
assertThat(requestHandler.handleExitRequestCalled).isTrue();
}
private static class TestExitRequestHandler implements ExitRequestHandler {
boolean handleExitRequestCalled;
@Override public void handleExitRequest(int status) {
handleExitRequestCalled = true;
}
}
}
Мне было трудно писать этот пост в блоге, потому что высказанные мною пункты казались слишком очевидными и хорошо понятными. Но, читая код (как мой, так и написанный другими), у меня сложилось впечатление, что, хотя мы понимаем концепцию «составление по наследованию», применить ее на практике все же непросто.
Я слишком придирчив? Я надеюсь, что нет (хотя я признаю, что эта проблема меня раздражала в течение некоторого времени, у меня наконец-то появилась возможность пожаловаться.) IHMO, каждая деталь имеет значение при написании чистого кода 🙂
Обратная связь всегда приветствуется!