Вместо вступления
За многолетнюю карьеру и до сих пор, мне довольно часто приходится сталкиваться с типичными "болячками" разного уровня программистов, и как показал мой опыт, описанные ниже кейсы встречаются довольно часто, почти во всех проектах. Да, возможно мне не повезло, и именно мне приходится лицезреть повторяющиеся из раза в раз приемы, которые в результате приводят к плохо поддерживаемому коду. Т. е. такой код действительно работает, и выполняет поставленную задачу, однако обладает определенной связностью, так что даже сам автор тратит уйму времени на его поддержку. В этой статье я постараюсь разобрать часть из них. Цель - поделиться своим опытом, а так же получить обратную связь от уважаемой аудитории.
Абстрактные классы
В общем случае наблюдается следующая картина -
Стрелочки 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)).
К сожалению, человек оказался со непростым характером, и выразил недоумение по поводу вопроса в целесообразности такой конструкции. Впрочем там весь код пестрил подобными отсортированными массивами.
Старайтесь все-таки прислушиваться к мнению коллег и развиваться. Опыт показывает, что лучше работать с лояльными людьми, пусть и не самой высокой квалификации, нежели с сумрачными гениями.
Всем добра.
