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

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

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

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

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

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

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

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

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

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

  

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

  

• есть обе стрелочки - а надо ли? в 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<String, String> cache;
  
  public DbReader(JdbcTemplate jdbcTemplate, Cache<String, String> 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<String, String> cache;
  
  public DbReader(JdbcTemplate jdbcTemplate, Cache<String, String> 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<String, String> cache;
  
  public DbReader(DbReader peer, Cache<String, String> 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<String, Object> 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

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

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

Заключение

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

P. S.

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

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

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

Всем добра.