Частые и не очень грабли ООП проектирования на Java05.01.2024 15:45

Вместо вступления

За многолетнюю карьеру и до сих пор, мне довольно часто приходится сталкиваться с типичными «болячками» разного уровня программистов, и как показал мой опыт, описанные ниже кейсы встречаются довольно часто, почти во всех проектах. Да, возможно мне не повезло, и именно мне приходится лицезреть повторяющиеся из раза в раз приемы, которые в результате приводят к плохо поддерживаемому коду. Т. е. такой код действительно работает, и выполняет поставленную задачу, однако обладает определенной связностью, так что даже сам автор тратит уйму времени на его поддержку. В этой статье я постараюсь разобрать часть из них. Цель — поделиться своим опытом, а так же получить обратную связь от уважаемой аудитории.

Абстрактные классы

В общем случае наблюдается следующая картина —

b5177fd3e79e22f76a7523eef6e3185e.png

Стрелочки uses обозначены разными цветами для удобства описания, дабы не постить много одинаковых картинок из их различных комбинаций.

Для начала, пройдемся по вырожденным ситуациям, где

  • отсутствует common part — да, такое встречается редко, но бывает; очевидно, что это просто интерфейс, а не абстрактный класс

  • отсутствует abstract part — и такое бывает; тогда зачем нужен вообще абстрактный класс, ибо в сущности это обычный класс

Далее случаи с uses

  • есть только красная стрелочка (слева) — классика жанра, паттерн Abstract Template; впрочем, такая конструкция прекрасно раскладывается в композицию вида

    4e25af2e04ea6ecfe7e24031f2b64856.png

  • есть только оранжевая стрелочка (справа) — встречается в Bridge, такая конструкция раскладывается в композицию другого вида

    153b665d88da5cc1b37b23908cf6ac68.png

  • есть обе стрелочки -, а надо ли? в 90% случаев получается макаронный код, где надо прыгать от абстрактного класса в конкретный несколько раз; разложить такое в ациклический граф уже не получится, имеем циклическую зависимость через имплементации

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

Прежде чем задумываться о иерархии, следует подумать о композиции, как писал дядя Джошуа Блох — предпочитайте композицию наследованию.

Наследование привносит сильную связность. Ах да, поговорим о композиции —

Композиция

Рассмотрим следующий пример — 

class DbReader {
  private final JdbcTemplate jdbcTemplate;
  
  public DbReader(JdbcTemplate jdbcTemplate) {
    this.jdbcTemplate = jdbcTemplate;
  }

  public String readInfo(String id) {
    // getting info from db by id
  }
  
}

Представим, что нам понадобилось сделать имплементацию, которая кеширует результат на какое-то время. Решаем в лоб — 

class DbReader {
  private final JdbcTemplate jdbcTemplate;
  private final Cache cache;
  
  public DbReader(JdbcTemplate jdbcTemplate, Cache cache) {
    this.jdbcTemplate = jdbcTemplate;
    this.cache = cache;
  }

  public String readInfo(String id) {
    return cache.putIfAbsent(id, key -> {
      // getting info from db by key  
    });
  }
  
}

Любители наследования сделают так — 

class DbReaderCached extends DbReader {
  private final Cache cache;
  
  public DbReader(JdbcTemplate jdbcTemplate, Cache cache) {
    super(jdbcTemplate);
    this.cache = cache;
  }

  @Override
  public String readInfo(String id) {
    return cache.putIfAbsent(id, key -> {
      super.readInfo(id);
    });
  }
  
}

Здесь уже появляется позможность сделать polymorphic substitution, ура. Но нужно ли тут наследование вообще? Почему бы не расцепить имплементации следующим образом

interface DbReader {
  String readInfo(String id);
}

class DbReaderCached implements DbReader {
  private final DbReader peer;
  private final Cache cache;
  
  public DbReader(DbReader peer, Cache cache) {
    this.peer = peer;
    this.cache = cache;
  }

  @Override
  public String readInfo(String id) {
    return cache.putIfAbsent(id, key -> {
      peer.readInfo(id);
    });
  }
  
}
// client code:
// DbReader readerDirect = new DbReaderDirect(new JdbcTemplate(...));
// DbReader dbReader = new DbReaderCached(readerDirect, new Cache(...));

Таким образом появляется четкое разделение функционала без наследования, которое можно делать один раз для одного класса (я про extends), в отличие от назначенного поведения через интерфейсы.

Инкапсуляция уровня конструктора

Многие, наверное, знают, что конструктор — это «грязный» код, наряду со статическими классами (туда же и паттерн singleton) и оператором if в бизнес логике. При правильном проектировании такие участки кода прячут в абстрактные или не очень (creator, information expert), фабрики. Поэтому, на самом деле все равно, какие и сколько (до разумных пределов) объектов нужно классу для его правильного функционирования. Достаточно часто разработчики пытаются закрыть детали конструирования объекта через комбинацию совершенно инородных объектов. Рассмотрим на примере — 

class Coord {
  private final int x;
  private final int y;
  
  Coord(ExtraEntity1 entity1, ExtraEntity2 entity2, ....){
    // conversion logic
    int a = entity1.getA();
    int b = entity2.getB();
    int c = entity2.getNext().getC();
    //
    
    this.x = a;
    this.y = b + c;
  }
  //...
}

Ну что же, прекрасно. А теперь, чтобы создать этот класс, надо что? правильно, надо проделать реверс инжиниринг «conversion logic», и тогда, и только тогда мы сможем подложить в конструктор нечто ожидаемое, что даст нам правильный объект. Решением по-лучше было бы сделать конструктор, принимающий именно то, что нужно этому классу, а еще лучше сделать фабричный метод of, а именно — 

class Coord {
  private final int x;
  private final int y;


  Coord(int x, int y) {
    this.x = x;
    this.y = y;
  }
  
  public static Coord of(ExtraEntity1 entity1, ExtraEntity2 entity2, ....){
    // conversion logic
    int a = entity1.getA();
    int b = entity2.getB();
    int c = entity2.getNext().getC();
    //
    
    return new Coord(a, b + c);
  }
  //...
}

Теперь, если и когда понадобится унести логику создания этой сущности в фабрику, то сделать это будет стоить ровно — вырезать+вставить. А чистый конструктор позволяет безболезненно и понятно создавать объект для целей тестирования и отладки (и не только).

В дополнение про тестирование — неплохо бы в целом задумываться о том, что если код написан так, что становится невозможным и\или труднодостижимым протестировать связку объектов, создавая их через оператор new, то скорее всего такой код написан плохо. Я понимаю, что spring творит чудеса, и подсунет все, что вам надо из контекста, однако невозможность тестирования вышеописанным методом только малой структуры объектов, в отвязке от спринга — плохой знак.

JS стиль

Нередко можно увидеть и такое — 

void method(Map params) {
  int x = (Integer)params.get(INT_X_PARAM);
  //....
}

или такое 

void method(Object param) {
  if(param instanceof Integer) {
    //...
  } else if(params instanceof AnotherType) {
    //...
  }
}

тут, думаю, все понятно. В первом случае напрашивается класс\pojo, описывающий параметры в явном виде. Во втором — на самом деле напрашивается набор перегруженных методов. Все таки java язык со статической типизацией, и этим преимуществом нужно пользоваться — отлавливать ошибки на раннем связывании — хорошее дело.

Var

Весьма спорная фича, мне видится ее применение лишь в наиболее очевидных случаях типа 

 for(var i = 0; i < 10; i++) {...}

В случаях работы с через var, в Intellij Idea, экстракция метода или переменной работает в лоб — т е подтянет конкретный тип с дженериком, который придется править ручками. Да и в целом, возрастает нагрузка на читающего код (а как известно, код читается 80% времени) — каждый раз подсматривая актуальный тип, каждый член команды тратит определенное время. Если код пишется лишь для себя — наверное было бы все равно.

final внутри методов

Здесь я бы сослался на книгу clean code

9b94e95af2a63f7af1e5f34d48097110.png

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

И да, для полей класса — ситуация обратная — лучше стремиться к иммутабельности и к преимуществам+гарантиям по jmm ключевого слова final. Однако и тут есть исключение — для DTO объектов эту рекомендацию можно ослабить в силу различных требований по сериализации таких объектов.

Заключение

Надеюсь данная статья поможет разработчикам пересмотреть некоторые типовые подходы к написанию кода.

P. S.

В одной из компаний мне довелось увидеть довольно интересный экземпляр, который был написан аж техлидом. В сущности человек изобрел hashmap с доступом O (log (n)).

К сожалению, человек оказался со непростым характером, и выразил недоумение по поводу вопроса в целесообразности такой конструкции. Впрочем там весь код пестрил подобными отсортированными массивами.

976c462ec5fb5ed13edf7c548ddb8020.png

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

Всем добра.

© Habrahabr.ru