# Как шаблонный метод может сломать ваш Java код
ООП — это замечательно\. За несоблюдение этой парадигмы принято ругать, а знание паттернов зачастую является обязательным\. Но даже правильный подход не страхует полностью от ошибок\. О том, как сломать программу при помощи обычного шаблонного метода, мы сегодня и узнаем\.
## Предисловие
Это ещё одна статья, родившаяся из проверки 24\-й версии [DBeaver](https://github.com/dbeaver/dbeaver) статическим анализатором PVS\-Studio\. В её ходе я нашёл несколько подозрительных мест, которые зацепили меня достаточно для того, чтобы я посвятил им по отдельной статье\. Обновляемый список статей из этого цикла:
* [Ловушки volatile, DCL и синхронизации в Java](https://pvs-studio.ru/ru/blog/posts/java/1128/)
* Как шаблонный метод может сломать вашу программу \(вы находитесь здесь\)
* [Побитовые проверки в Java и почему они так неоднозначны](https://pvs-studio.ru/ru/blog/posts/java/1135/)
## ООП? Шаблонный метод?
### ООП\!
Не так давно я писал [статью](https://pvs-studio.ru/ru/blog/posts/java/1103/) о том, как можно применять ООП в своей ежедневной работе\. Статья теоретическо\-практическая и заглядывает в вопрос шире, чем мы это сделаем сегодня\. Интересующихся темой приглашаю в ту статью, но весь необходимый материал мы повторим здесь — об этом можно не беспокоиться\. В рамках же этой статьи необходимость использования ООП и следования SOLID предлагаю взять за постулат\.
### Шаблонный метод\!
Собственно, один из самых простых паттернов — шаблонный метод\. Но если кто\-то вдруг забыл его реализацию, или вы ещё новичок, то давайте повторим его содержание\.
В этом разделе будет базовая информация, так что если вы с ней знакомы, то можете переходить сразу к следующему\. Остальных же приветствую в теоретической части\.
Сразу отдаём дань GoF и берём классическую схему этого паттерна из их [книги](https://en.wikipedia.org/wiki/Design_Patterns):
Вот настолько просто\. Если вы умеете читать классовые диаграммы, конечно\. В противном случае расшифруем через пример в коде, который хочет казаться реальным\.
Допустим, нам надо выводить в консоль \(хотя это может быть и файл, и что угодно ещё\) содержимое класса _Person_, который содержит в себе только имя и фамилию\.
Внутренности класса
```cpp
public class Person {
private final String name;
private final String surname;
public String getName() {
return name;
}
public String getSurname() {
return surname;
}
public Person(String name, String surname) {
this.name = name;
this.surname = surname;
}
}
```
Если вам показалось, что я не хочу переусложнять, вам не показалось :\)
Просто так содержимое класса мы вывести не можем, нам надо его сериализовать\. Для интереса представим, что формата сериализации два: json и xml\. Мы бы, конечно, могли сделать всё в одном классе, а нужный тип сериализации выбирать посредством какого\-нибудь перечисления, но тогда мы бы нарушили два принципа SOLID:
* SRP: совместив логику разных сериализаторов в одном, мы нарушим принцип единственной ответственности;
* OCP: добавляя новые типы сериализации в будущем, мы нарушим принцип открытости\-закрытости, так как придётся менять уже существующий класс\.
Вспомнив об этом, разумеется, сразу же понимаем, что так делать не надо\. Вместо этого определим абстрактный метод\-сериализатор в классе нашего принтера\. Реализация класса тривиальна:
```cpp
public abstract class AbstractPersonPrinter {
protected abstract String serialize(Person person);
public void print(Person person) {
System.out.println(serialize(person));
}
}
```
Всё, что нам осталось, это создать наследников, которые будут реализовывать каждый свою логику\. Для json:
```cpp
public class JsonPersonPrinter extends AbstractPersonPrinter {
@Override
public String serialize(Person person) {
var sb = new StringBuilder();
var s = System.getProperty("line.separator");
sb.append("{").append(s);
sb.append(" name: \"").append(person.getName())
.append("\"")
.append(s);
sb.append(" surname: \"").append(person.getSurname())
.append("\"")
.append(s);
sb.append("}");
return sb.toString();
}
}
```
И для xml:
```cpp
public class XmlPersonPrinter extends AbstractPersonPrinter {
@Override
public String serialize(Person person) {
var sb = new StringBuilder();
var s = System.getProperty("line.separator");
sb.append("").append(s);
sb.append(" ").append(s);
sb.append(" ").append(person.getName())
.append(s);
sb.append(" ").append(s);
sb.append(" ").append(s);
sb.append(" ").append(person.getSurname())
.append(s);
sb.append(" ").append(s);
sb.append("");
return sb.toString();
}
}
```
Вуаля\. Теперь мы можем сконфигурировать какой\-нибудь логгер и получать вывод в желаемом формате\. Если вам когда\-то вообще хотелось получать вывод в json или xml\.
Код какого\\\-нибудь логгера
```cpp
public class ConsoleLogger {
private final AbstractPersonPrinter printer;
public ConsoleLogger(AbstractPersonPrinter printer) {
this.printer = printer;
}
public void logPerson(Person person) {
printer.print(person);
}
....
}
```
Вывод какого\\\-нибудь логгера
json:
```cpp
{
name: "John"
surname: "Doe"
}
```
xml:
```cpp
John
Doe
```
Вернёмся к диаграмме класса\. Её можно перерисовать под наш пример, чтобы было проще понять предыдущую:
Хорошо, затянутое объяснение паттерна закончено\. Почти\. Только упомяну, что если у вас были мысли, что вместо наследования тут может быть лучше использовать композицию \(то есть стратегию\) — то вы в своём праве\. Я же просто хотел продемонстрировать конкретный паттерн, так что условился использовать именно его\.
## Реальная практика
### Задача и решение
О шаблонном методе можно сказать как о базовом во многих смыслах, но в нём есть и неочевидные места\.
Вот, например, была следующая задача: на веб\-странице есть форма с данными, большую часть из которых можно менять\. Причём нужно запоминать их версию до и после изменений, чтобы при закрытии страницы уведомлять пользователя о том, что он их не сохранил\. Таких форм на разных страницах много, поэтому нужно общее решение\.
Пробуем решить задачу:
1. Делаем абстрактный обобщённый класс _ParameterBase_, который заключает в себе вышеописанную логику\. Копирование из DTO и других объектов происходит через рефлексию, а логика сохранения и обновления состояния реализована с помощью паттерна хранитель \(memento\);
1. Такой подход довольно грубый, поэтому пока не останавливаемся\. Мы не учли как сложные вещи вроде маппинга полей с разными типами данных \(опустим для простоты\), так и простые вроде простого игнорирования полей исходного объекта для автоматического копирования\.
Проблему во втором пункте надо как\-то решать\. Для этого вводим переопределяемый метод, в котором можно указывать ненужные поля\. Вот немного упрощённое решение, которое я тогда написал:
```cpp
public abstract class ParameterBase
: ObservableValidator where TEntity : class
{
protected virtual List GetIgnoredProperties()
=> new List();
public ParameterBase(TEntity entity)
{
var ignore = GetIgnoredProperties();
var sourceProp = GetType().GetProperties();
var colNumber = sourceProp.Length - ignore.Length;
var colCounter = 0;
foreach (var prop in sourceProp)
if (!(ignore.Contains(prop.Name)))
{
prop.SetValue(this, typeof(TEntity)
.GetProperty(prop.Name, Consts.PropertyBindingFlags)
.GetValue(entity, null), null);
colCounter++;
}
if (colNumber != colCounter)
throw new InvalidOperationException(
"Not every parameter field got its value");
this.PropertyChanged += Change;
}
....
}
```
Код выше какой\\\-то странный
Это не у меня Java сломалась, это С\# код :\) Прошу воспринимать это как стилистическое отступление, код я постарался упростить\.
На резонный вопрос о том, почему во фронтэнд задаче используется C\#, отвечу, что это фреймворк Blazor, который активно [используется](https://pvs-studio.ru/ru/blog/posts/csharp/1023/) в нашей компании\.
К слову, раз мы заговорили о других языках программирования, то у нас есть [статья](https://pvs-studio.ru/ru/blog/posts/cpp/1125/) на схожую тему и для C\+\+\.
Алгоритм довольно простой: копируем при помощи рефлексии все свойства из обобщённой модели в потомка _ParameterBase_, игнорируя указанные в самом потомке\. Если что\-то не сошлось по количеству, то выбрасываем исключение\. Собственно, метод _GetIgnoredMappingProperties_ и является частным случаем шаблонного метода, который называется **зацепка** \(hook\)\. Задача решена\. Всё хорошо, так ведь?
### Внезапное предупреждение
Всё бы хорошо, но после сборки срабатывает инкрементальный анализ, и PVS\-Studio выдаёт следующее сообщение на код выше:
[V3068](https://pvs-studio.ru/ru/docs/warnings/v3068/) Calling overrideable class member 'GetIgnoredProperties' from constructor is dangerous\. ParameterBase\.cs\(34\)
И вот тут надо сказать, что я не очень люблю анализ на [code smell](https://ru.wikipedia.org/wiki/%D0%9A%D0%BE%D0%B4_%D1%81_%D0%B7%D0%B0%D0%BF%D0%B0%D1%88%D0%BA%D0%BE%D0%BC)\-ы\. В большинстве случаев хочется встать в позу, пожурить инструмент и с умным видом [подавить](https://pvs-studio.ru/ru/docs/manual/0017/) предупреждение\. И не буду нагнетать ненужной интриги — в данном случае анализатор не прав, никакой проблемы в моём решении нет\. И именно так я в своё время и поступил, занеся предупреждение в _suppress_ файл проекта, откуда я его сейчас и достал\.
Конечно, можно пофантазировать о возможной проблеме и её исправлении:
1. Если добавить в потомка конструктор, принимающий дополнительные данные, и в зависимости от этих данных менять поведение _GetIgnoredProperties_, то мы получим именно ту проблему, о которой говорится в описании диагностики;
1. Для исправления можно было бы сделать конструктор приватным, инициализацию вынести в отдельный метод, а созданием объектов управлять через фабрику\.
Но зачем заниматься этой эквилибристикой только ради того, чтобы анализатор от тебя отстал, если проще подавить предупреждение? В общем, неинтересно\.
## У нас проблемы
### Опасность близко
Об этом случае я и вспомнил, когда, листая DBeaver, наткнулся на аналогичное для Java сообщение PVS\-Studio:
[V6052](https://pvs-studio.ru/ru/docs/warnings/v6052/) Calling overridden 'isBinaryContents' method in 'TextWithOpen' parent\-class constructor may lead to use of uninitialized data\. Inspect field: binary\. TextWithOpenFile\.java\(77\), TextWithOpen\.java\(59\)
Вспоминая прошлый опыт, я намеревался пройти дальше, но всё же решил изучить код\. Итак, метод _isBinaryContents_ находится в классе _TextWithOpenFIle_:
```cpp
public class TextWithOpenFile extends TextWithOpen {
private final boolean binary;
....
@Override
protected boolean isBinaryContents() {
return binary;
}
....
}
```
Не выглядит захватывающе\. Но давайте посмотрим на единственное место, где он используется:
```cpp
public TextWithOpen(Composite parent, boolean multiFS, boolean secured) {
super(parent, SWT.NONE);
....
if (!useTextEditor && !isBinaryContents()) {
....
editItem.setEnabled(false);
}
....
}
```
Анализатор указал на огромный конструктор, который я для удобства сократил\. Вышеупомянутый _isBinaryContents_ используется в условии, в теле которого я вырезал около 40 строк\. Имейте в виду, что сейчас мы находимся в классе\-родителе _TextWithOpen_\. Теперь бы посмотреть, что находится внутри родительского _isBinaryContents_:
```cpp
protected boolean isBinaryContents() {
return false;
}
```
О, та самая **зацепка**, о которой мы говорили ранее\. Получается, разработчики хотели, чтобы в родительском классе второе условие всегда равнялось _true_ \(не забываем про отрицание в нём\)\. Так, а о чём говорит [документация](https://pvs-studio.ru/ru/docs/warnings/v6052/) к диагностике?
> Выявлен случай использования метода в родительском конструкторе, который, в свою очередь, переопределяется в дочернем классе\. Такое использование может привести к тому, что переопределённый метод будут использовать неинициализированные поля класса\.
Выходит, надо проверить конструктор класса _TextWithOpenFIle_:
```cpp
public TextWithOpenFile(
Composite parent,
String title,
String[] filterExt,
int style,
boolean binary,
boolean multiFS,
boolean secured
) {
super(parent, multiFS, secured); // <=
this.title = title;
this.filterExt = filterExt;
this.style = style;
this.binary = binary; // <=
}
```
Ого\. Ошибка\. Сначала мы вызываем конструктор _TextWithOpenFile_, тот в свою очередь вызывает конструктор _TextWithOpen_, где вызывается _isBinaryContents_, читающий значение _binary_, которое по умолчанию является _false_\. И только **после** этого поле _binary_ инициализируется в конструкторе _TextWithOpenFile_\.
Самое неприятное, что сходу непонятно, как это исправить\. Просто взять и переместить вниз вызов _super_, к сожалению, не выйдет :\)
Проще всего будет вынести инициализацию в отдельный метод и отдельно звать его во всех конструкторах\. Просто и сердито, но не оптимально: если переусложнить инициализацию, то вероятность в ней ошибиться при создании нового наследника будет выше\.
Хорошей альтернативой было бы использовать порождающие паттерны:
* Любая из фабрик, как я уже упоминал, поместила бы процесс создания объектов в отдельное место\. Тогда рядом хотя бы будут напоминания о необходимости инициализации и её способе;
* Для данного случая также поможет строитель — классическое решение для ситуаций, когда инициализация становится слишком сложной\. С помощью этого паттерна можно разбить её на несколько более простых шагов, тем самым упростив дальнейшее расширение\.
### Мораль
Кажется, где\-то здесь надо признать, что мой скептицизм был посрамлён, а предупреждения анализатора игнорировать нельзя\. Но я всё же останусь при своём мнении\. В конце концов, даже в нашей [документации](https://pvs-studio.ru/ru/docs/warnings/v3068/) написано:
> В случае, если вы точно уверены, что вам нужно описанное поведение при инициализации объекта и вы хотите скрыть предупреждение от анализатора, пометьте сообщение как ложнопозитивное\.
В моём случае я оценивал и оцениваю вероятность возможной ошибки как невероятно малую, хотя и без гарантий\. Здесь же мы имеем значительно более сложную инициализацию с большим количеством параметров, и в этих обстоятельствах опасный приём закономерно привёл к ошибке\. В общем, здравая оценка рисков — главная вещь при принятии решений\. Тоже не звучит захватывающе, я знаю :\)
## Послесловие
И на этом хочется перейти к заключению\. Надеюсь, вам было интересно узнать, как следование правильному подходу может привести к обидной ошибке\. И если вы будете лишний раз всё перепроверять, используя переопределяемые методы в конструкторах, то я своё дело сделал :\)
Если вам хочется поискать эту или другие ошибки в своём проекте, то вы можете бесплатно попробовать PVS\-Studio, перейдя по [этой](https://pvs-studio.ru/ru/pvs-studio/try-free/) ссылке\.
А ещё я напомню, что это не все интересные вещи, которые были найдены в DBeaver\. Ссылки на другие статьи можно найти в начале, а чтобы не пропустить выход новых статей, предлагаю подписаться на мой [Х\-твиттер](https://x.com/kvolokhovskii)\.