[Из песочницы] Размышления на тему ООП и состоянии объектов
Программный код пишется сверху вниз. В том же порядке читаются и выполняются инструкции. Это логично и все давно к этому привыкли. В некоторых случаях можно поменять порядок операций. Но иногда очерёдность вызовов функций важна, хотя синтаксически это неочевидно. Получается, что код выглядит рабочим, даже после перестановки вызовов, а результат оказывается неожиданным.
Однажды подобный код попался мне на глаза.
Проблема
Однажды, ковыряясь в чужом коде в совместном проекте, а обнаружил функцию наподобие:
public Product fill(Product product, Images images, Prices prices, Availabilities availabilities){
priceFiller.fill(product, prices); //do not move this line below availabilityFiller call, availabilities require prices
availabilityFiller.fill(product, availabilities);
imageFiller.fill(product, images);
return product;
}Конечно сразу в глаза бросается не самый «элегантный» стиль: классы хранят данные (POJO), функции изменяют входящие объекты…
В целом вроде бы ничего. У нас есть объект, в котором не хватает данных, и есть сами данные, пришедшие из других источников (сервисов), которые мы теперь в этот объект и поместим, чтобы сделать его полноценным.
Но есть и некоторые нюансы.
- Мне не нравится, что ни одна функция не написана в стиле ФП и изменяет объект, переданный как аргумент.
- Но допустим, что это было сделано, чтобы сократить время обработки и количество создаваемых объектов.
- Только комментарий в коде говорит о том, что последовательность вызовов важна, и при встраивании нового
Fillerнадо быть осторожным- А ведь количество людей, работающих над проектом больше 1 и не все знают об этой хитрости. Особенно новые люди в команде (не обязательно на предприятии).
Последний пункт меня особенно насторожил. Ведь API построена так, что мы не знаем, что изменилось в объекте, после вызова данной функции. Например у нас до и после есть метод Product::getImages, который до вызова функции fill выдаст пустой список, а после список с картинками к нашему продукту.
С самими Filler дела обстоят ещё хуже. AvailabilityFiller никак не даёт понять, что ожидает, что информация о цене товара уже заложена в передаваемых объект.
И вот я задумался о том, как бы я мог оградить своих коллег от ошибочного использования функций.
Предложенные решения
Сперва, я решил обсудить данный случай со своими коллегами. К сожалению, все предложенные ими решения, не показались мне правильным подходом.
RuntimeException
Одним из предложенных вариантов был:, а ты в AvailabilityFiller впиши в начале функции Objects.requireNonNull(product.getPrices) и тогда любой программист во время локальных тестов уже получит ошибку.
- но цены и правда может не быть, если сервис был недоступен или ещё какая ошибка, тогда товар должен получить статус «не в наличии». Придётся приписывать всякие флаги или прочее, чтобы отличить «нет данных» от «даже не запрашивали».
- Если же бросить исключение в самом
getPrices, тогда мы создадим те же проблемы, что и современная Java со списками- Допустим в функцию передаётся List, предлагающий в своей API метод get… Я знаю, что не надо менять передаваемые объекты, а создавать новые. Но суть в том, что API нам такой метод предлагает, но в рантайме может вылететь ошибка, если это неизменяемый список, как например полученный из Collectors.toList ()
- Если же бросить исключение в самом
- если
AvailabilityFillerбудет кем-то использован в другом месте, то программист написавший вызов, не сразу поймёт, в чём проблема. Только после запуска и Дебага. Потом ему ещё придётся разбираться в коде, чтобы понять, откуда взять данные.
Test
«А ты напиши тест, который будет ломаться, если поменять очерёдность вызовов». т.е. если все Filler будут возвращать «новый» продукт, получится что-то наподобие:
given(priceFillerMock.fill(eq(productMock), any())).willReturn(productWithPricesMock);
given(availabilityFillerMock.fill(eq(productMockWithPrices), any())).willReturn(productMockWithAvailabilities);
given(imageFillerMock.fill(eq(productMockWithAvailabilities), any())).willReturn(productMockWithImages);
var result = productFiller.fill(productMock, p1, p2, p3);
assertThat("unexpected return value", result, is(productMockWithImages));- не люблю тесты, которые настолько «White-Box»
- Ломается при каждом новом
Filler - Ломается при смене очередности независимых вызовов
- Опять таки не решает проблему переиспользования самого
AvailabilityFiller
Собственные попытки решения проблемы
Идея
Думаю, что вы уже догадались, что я бы хотел решить проблему на уровне компиляции. Ну зачем мне, спрашивается, компилируемый язык со строгой типизацией, если я не могу предотвратить ошибку.
И я задумался, принадлежат ли объект без дополнительных данных и «расширенный» объект к одному и тому же классу?
Разве не будет правильным описать отдельными классами или интерфейсами различные возможные состояния объекта?
Таким образом моя идея состояла в следующем:
public Product fill( product, Images images, Prices prices, Availabilities availabilities){
var p1 = priceFiller.fill(product, prices);
var p2 = availabilityFiller.fill(p1, availabilities);
return imageFiller.fill(p2, images);
}
PriceFiller
public ProductWithPrices fill( product, Prices prices)
AvailabilityFiller
public ProductWithAvailabilities fill( product, Prices prices)
или
public fill( product, Prices prices) Т.е. задаваемый изначально продукт является инстансом класса отличного от возвращаемого, чем уже показаны изменения данных.
Filler'ы же в своих API точно указывают, какие данные им нужны и что они возвращают.
Таким образом можно предотвратить неправильную очередность вызовов.
Реализация
Как такое воплотить в реальность в Java? (Напомню, что наследование от нескольких классов в Java невозможно.)
Сложность добавляют независимые операции. Например, картинки могут быть добавлены и до и после добавления цен, а так же в самом конце функции.
Тогда могут быть
class ProductWithImages extends BaseProduct implements ImageAware{}
class ProductWithImagesAndPrices extends BaseProduct implements ImageAware, PriceAware{}
class Product extends BaseProduct implements ImageAware, PriceAware, AvailabilityAware{}Как это всё описать?
Создавать адаптеры?
public ProductWithImagesAndPrices( base){
this.base = base;
this.images = Collections.emptyList();
}
public long getId(){
return this.base.getId();
}
public Price getPrice(){
return this.base.getPrice();
}
public List getImages(){
return this.images;
} Копировать данные/ссылки?
public ProductWithImagesAndPrices( base){
this.id = base.getId();
this.prices = base.getPrices();
this.images = Collections.emptyList();
}
public List getImages(){
return this.images;
} Как уже заметно, всё сводится просто к огромнейшему количеству кода. И это при том, что в примере оставил только 3 вида входных данных. В реальном мире их может быть намого больше.
Получается, что затраты на написание и поддержку такого кода себя не оправдывают, хотя сама идея разделения состояния на отдельные классы мне показалась очень привлекательной.
Отступление
Если посмотреть на другие языки, то где-то эту проблему решить проще, а где-то нет.
Например в Go можно прописать ссылку на расширяемый класс без «копирования» или «перегрузки» методов. Но речь не о Go
Ещё одно отступление
Пока писал эту статью, пришло в голову ещё одно возможное решение с Proxy, позвалающее прописывать только новые методы, но требующее иерархии интерфейсов. В общем страшное, сердитое и не подходящее. Если вдруг кому-то интересно:
public class Application {
public static void main(String[] args) {
var baseProduct = new BaseProductProxy().create(new BaseProductImpl(100L));
var productWithPrices = fillPrices(baseProduct, BigDecimal.TEN);
var productWithAvailabilities = fillAvailabilities(productWithPrices, "available");
var productWithImages = fillImages(productWithAvailabilities, List.of("url1, url2"));
var product = productWithImages;
System.out.println(product.getId());
System.out.println(product.getPrice());
System.out.println(product.getAvailability());
System.out.println(product.getImages());
}
static ImageAware fillImages(T base, List images) {
return (ImageAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{ImageAware.class, BaseProduct.class},
new MyInvocationHandler<>(base, new ImageAware() {
@Override
public List getImages() {
return images;
}
}));
}
static PriceAware fillPrices(T base, BigDecimal price) {
return (PriceAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{PriceAware.class},
new MyInvocationHandler<>(base, new PriceAware() {
@Override
public BigDecimal getPrice() {
return price;
}
}));
}
static AvailabilityAware fillAvailabilities(PriceAware base, String availability) {
return (AvailabilityAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{AvailabilityAware.class},
new MyInvocationHandler<>(base, new AvailabilityAware() {
@Override
public String getAvailability() {
return base.getPrice().intValue() > 0 ? availability : "sold out";
}
}));
}
static class BaseProductImpl implements BaseProduct {
private final long id;
BaseProductImpl(long id) {
this.id = id;
}
@Override
public long getId() {
return id;
}
}
static class BaseProductProxy {
BaseProduct create(BaseProduct base) {
return (BaseProduct) Proxy.newProxyInstance(this.getClass().getClassLoader(), new Class[]{BaseProduct.class}, new MyInvocationHandler<>(base, base));
}
}
public interface BaseProduct {
default long getId() {
return -1L;
}
}
public interface PriceAware extends BaseProduct {
default BigDecimal getPrice() {
return BigDecimal.ZERO;
}
}
public interface AvailabilityAware extends PriceAware {
default String getAvailability() {
return "sold out";
}
}
public interface ImageAware extends AvailabilityAware {
default List getImages() {
return Collections.emptyList();
}
}
static class MyInvocationHandler implements InvocationHandler {
private final U additional;
private final T base;
MyInvocationHandler(T base, U additional) {
this.additional = additional;
this.base = base;
}
@Override
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
if (Arrays.stream(additional.getClass().getInterfaces()).anyMatch(i -> i == method.getDeclaringClass())) {
return method.invoke(additional, args);
}
var baseMethod = Arrays.stream(base.getClass().getMethods()).filter(m -> m.getName().equals(method.getName())).findFirst();
if (baseMethod.isPresent()) {
return baseMethod.get().invoke(base, args);
}
throw new NoSuchMethodException(method.getName());
}
}
} Вывод
Что же получается? С одной стороны интересный подход, применяющий к «объекту» в разных состояниях отдельные классы и гарантирующий этим предотвращение ошибок, вызванных неправильной последовательностью вызовов методов, изменяющих этот объект.
С другой стороны, такой подход заставляет писать столько кода, что от него сразу хочется отказаться. Нагромождение интерфейсов и классов только мешает разобраться в проекте.
В другом своем проекте я всё же попробовал использовать такой подход. И сначала на уровне интерфейсов всё было просто отлично. Я написал функции:
List firstStep(List ts){}
List nStep(List ts){}
List finalStep(List ts){} Указав тем самым, что определённый шаг обработки данных требует дополнительной информации, которая не нужна ни в начале обратобки, ни в её конце.
Испльзуя mock'и, я сумел протестировать код. Но когда дело дошло до реализации и колличество данных и разных источников начало расти, я быстро сдался и переделал всё под «обычный» вид. Всё работает и никто не жалуется. Получается, что работоспособность и простота кода побеждают над «предотвращением» ошибок, а проследить за правильной последовательностью вызовов можно и вручную, пусть даже ошибка проявит себя только на этапе ручного тестирования.
Может, если бы я сделал шаг назад и посмотрел бы на код с другой стороны, у меня появились бы совсем другие решения. Но так вышло, что я заинтересовался именно этой прокомментированной сторокой.
Вот уже под конец статьи, задумавшись о том, что раз уж не красиво описывать setter’ы в интерфейсах, то можно представить сборку данных о продукте в виде Builder'а, который после добавления опрелённых данных возвращает другой интерфейс. Опять таки всё зависит от сложности логики построения объектов. Если вы работали с Spring Security, то вам знакомы такого рода решения.
Для моего примера выходит:
public class Application_2 {
public static void main(String[] args) {
var product = new Product.Builder()
.id(1000)
.price(20)
.availability("available")
.images(List.of("url1, url2"))
.build();
System.out.println(product.getId());
System.out.println(product.getAvailability());
System.out.println(product.getPrice());
System.out.println(product.getImages());
}
static class Product {
private final int price;
private final long id;
private final String availability;
private final List images;
private Product(int price, long id, String availability, List images) {
this.price = price;
this.id = id;
this.availability = availability;
this.images = images;
}
public int getPrice() {
return price;
}
public long getId() {
return id;
}
public String getAvailability() {
return availability;
}
public List getImages() {
return images;
}
public static class Builder implements ProductBuilder, ProductWithPriceBuilder {
private int price;
private long id;
private String availability;
private List images;
@Override
public ProductBuilder id(long id) {
this.id = id;
return this;
}
@Override
public ProductWithPriceBuilder price(int price) {
this.price = price;
return this;
}
@Override
public ProductBuilder availability(String availability) {
this.availability = availability;
return this;
}
@Override
public ProductBuilder images(List images) {
this.images = images;
return this;
}
public Product build(){
var av = price > 0 && availability != null ? availability : "sold out";
return new Product(price, id, av, images);
}
}
public interface ProductBuilder {
ProductBuilder id(long id);
ProductBuilder images(List images);
ProductWithPriceBuilder price(int price);
Product build();
}
public interface ProductWithPriceBuilder{
ProductBuilder availability(String availability);
}
}
} Так что:
- Пишите чистые функции
- Пишите красивый и понятный код
- Помните, что краткость — сестра и что главное, чтобы код работал
- Не стесняйтесь ставить вещи под вопрос, искать иные пути и даже забрасывать альтернативные решения, если уж на то пошло
- Не молчите! Во время объяснения проблемы другим, рождаются лучшие решения (Rubber Duck Driven Developent)
