Программный код пишется сверху вниз. В том же порядке читаются и выполняются инструкции. Это логично и все давно к этому привыкли. В некоторых случаях можно поменять порядок операций. Но иногда очерёдность вызовов функций важна, хотя синтаксически это неочевидно. Получается, что код выглядит рабочим, даже после перестановки вызовов, а результат оказывается неожиданным.
Однажды подобный код попался мне на глаза.
Проблема
Однажды, ковыряясь в чужом коде в совместном проекте, а обнаружил функцию наподобие:
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(<? extends BaseProduct> 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(<? extends BaseProduct> product, Prices prices)
AvailabilityFiller
public ProductWithAvailabilities fill(<? extends ProductWithPrices> product, Prices prices)
или
public <BaseProduct & PriceAware & AvailabilityAware> fill(<? extends BaseProduct & PriceAware> 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(<? extends BaseProduct & PriceAware> base){
this.base = base;
this.images = Collections.emptyList();
}
public long getId(){
return this.base.getId();
}
public Price getPrice(){
return this.base.getPrice();
}
public List<Image> getImages(){
return this.images;
}Копировать данные/ссылки?
public ProductWithImagesAndPrices(<? extends BaseProduct & PriceAware> base){
this.id = base.getId();
this.prices = base.getPrices();
this.images = Collections.emptyList();
}
public List<Image> 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 <T extends BaseProduct> ImageAware fillImages(T base, List<String> images) {
return (ImageAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{ImageAware.class, BaseProduct.class},
new MyInvocationHandler<>(base, new ImageAware() {
@Override
public List<String> getImages() {
return images;
}
}));
}
static <T extends BaseProduct> 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<String> getImages() {
return Collections.emptyList();
}
}
static class MyInvocationHandler<T extends BaseProduct, U extends BaseProduct> 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());
}
}
}Вывод
Что же получается? С одной стороны интересный подход, применяющий к "объекту" в разных состояниях отдельные классы и гарантирующий этим предотвращение ошибок, вызванных неправильной последовательностью вызовов методов, изменяющих этот объект.
С другой стороны, такой подход заставляет писать столько кода, что от него сразу хочется отказаться. Нагромождение интерфейсов и классов только мешает разобраться в проекте.
В другом своем проекте я всё же попробовал использовать такой подход. И сначала на уровне интерфейсов всё было просто отлично. Я написал функции:
<T extends Foo> List<T> firstStep(List<T> ts){}
<T extends Foo & Bar> List<T> nStep(List<T> ts){}
<T extends Foo> List<T> finalStep(List<T> 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<String> images;
private Product(int price, long id, String availability, List<String> 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<String> getImages() {
return images;
}
public static class Builder implements ProductBuilder, ProductWithPriceBuilder {
private int price;
private long id;
private String availability;
private List<String> 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<String> 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<String> images);
ProductWithPriceBuilder price(int price);
Product build();
}
public interface ProductWithPriceBuilder{
ProductBuilder availability(String availability);
}
}
}Так что:
- Пишите чистые функции
- Пишите красивый и понятный код
- Помните, что краткость — сестра и что главное, чтобы код работал
- Не стесняйтесь ставить вещи под вопрос, искать иные пути и даже забрасывать альтернативные решения, если уж на то пошло
- Не молчите! Во время объяснения проблемы другим, рождаются лучшие решения (Rubber Duck Driven Developent)
Спасибо за внимание
Комментарии (41)
thelongrunsmoke
05.12.2019 12:24В ООП, вызывать методы ради сайдэффектов это обычная практика, особенно в высокоуровневом коде. Однако, в целом, это классическая с ситуация, если важна последовательность или целостность инициализации объекта, то вы используете фабрику или билдер, в зависимости от того, где находятся нужные для работы данные. Билдер даже можно передавать как аргумент, и возвращать из метода.
Вариант с бросанием исключения тоже приемлем. Не все договорённости можно выразить в коде, поэтому создание исключения с информативным сообщением и которое сможет возникать только во время разработки — не такой плохой способ, ибо Fail-fast.
DarthVictor
06.12.2019 10:32Вариант с бросанием исключения тоже приемлем
В данном случае еще именованием можно обратить внимание.
Например переименовал метод у availabilityFiller с fill на что-то fillOnlyWithProductWithPrice. Такое длинное имя скорее бросится в глаза при рефакторинге и на него обратят внимание.
Еще есть вариант пересохранять объект product под другим именем
priceFiller.fill(product, prices); Product productWithPrice = product; availabilityFiller.fill(productWithPrice, availabilities); Product productWithPriceAndAvailability = productWithPrice; imageFiller.fill(productWithPriceAndAvailability, images);
Даже если компилятор не выкинет эти переменные потом, новых объектов мы не создадим.
Cryvage
05.12.2019 14:42Тут главный вопрос в том, почему «availabilityFiller» требует заполненных цен. Лучшим выходом, было бы избавиться от этой зависимости, переписав «availabilityFiller», а возможно и «Product».
Если же это невозможно, или слишком сложно, то самый простой выход — избавиться от «availabilityFiller», заменив его на «priceAndAvailabilityFiller», который требует одновременной передачи, как «Prices», так и «Availabilities». Ещё, понадобится метод: «Prices getPrices(Product product)».
Недостаток тут только один, и он вполне очевиден. В сценариях, когда «Prices» и «Availabilities» заполняются раздельно, «Prices» будет заполняться повторно. Но не зная всех деталей (как цены хранятся в продукте, как часто происходят подобные операции и т.д.), нельзя сказать, станет ли это узким местом.
elegorod
05.12.2019 15:50Получается, что код выглядит рабочим, даже после перестановки вызовов, а результат оказывается неожиданным.
А это уже проблема того, кто переставляет методы, не разобравшись сначала в коде. Как можно вообще что-то менять в коде просто так, не понимая, как он работает?
Придётся приписывать всякие флаги или прочее, чтобы отличить «нет данных» от «даже не запрашивали».
Можно сделать объект Prices, если «даже не запрашивали» он будет null, а если «нет данных», тогда будет подкласс EmptyPrices или просто цены с пустым контентом, не null. И тогда не надо даже писать Objects.requireNonNull(product.getPrices) — второй метод и так упадёт с NullPointerException, если не вызвать первый.
pin2t
05.12.2019 16:10Ну во-первых, ваши геттеры в классе ничего не делают, кроме того что разрушают инкапсуляцию. И поэтому они просто ненужны. Просто сделайте поля public. Бдет тоже самое но кратко и понятно.
Во-вторых, ваши Builder классы тоже не нужны, потому что по сути они вызывают конструктор, попутно излишне перекладывя переменные туда-сюда, делают лишнюю работу и делают её зря (точно также как геттеры). Надо просто сделать несколько конструкторов (да да, так можно!) с разным набором аргументов. и создавать ваш объект через new, как это и должно быть.
Итого:
public class Application_2 { public static void main(String[] args) { Product product = new Product(20, 1000, "available", List.of("url1, url2")); System.out.println(product.id); System.out.println(product.availability); System.out.println(product.price); System.out.println(product.images); } static class Product { public final int price; public final long id; public final String availability; public final List<String> images; public Product(int price, long id) { this(price, id, null); } public Product(int price, long id, String availability) { this(price, id, availability, null); } public Product(int price, long id, String availability, List<String> images) { this.price = price; this.id = id; this.availability = price > 0 && availability != null ? availability : "sold out"; this.images = images; } }
Гораздо короче и понятнее, а главное тотже самый функционал. Просто без лишней ненужной фигни, а так как задумано изначально в языке.
pin2t
05.12.2019 16:26Ну а если вы прям хотите задавать аргументы в любом порядке, можно сделать конструктор (о боже, четвертый!) из Map например
public class Application_2 { public static void main(String[] args) { Product product = new Product(20, 1000, "available", List.of("url1, url2")); Product product2 = new Product(new TreeMap<String, Object>() {{ put("id", new Integer(20); put("price", new Integer(1000)); put("availability", "available"); put("images", List.of("url1, url2")); }}; System.out.println(product.id); System.out.println(product.availability; System.out.println(product.price; System.out.println(product.images; } static class Product { public final int price; public final long id; public final String availability; public final List<String> images; public Product(int price, long id) { this(price, id, null); } public Product(int price, long id, String availability) { this(price, id, availability, null); } public Product(final Map<String, Object> fields) { if (fields.containsKey("availability") && !fields.containsKey("price")) throw new RuntimeException("product price is mandatory"); this((Integer)fields.get("price"), (Long)fields.get("id"), (String)fields.get("availability"), (List<String>)fields.get("images")); } public Product(int price, long id, String availability, List<String> images) { this.price = price; this.id = id; this.availability = price > 0 && availability != null ? availability : "sold out"; this.images = images; } }
Гораздо более объектно оринтированно чем ваши билдерыgeaker Автор
05.12.2019 19:27Спасибо за предложенный вариант.
Map наверное самый гибкий и удобный объект. Но опять таки слишком гибкий. Опечатки в ключах, захламление не используемыми данными и прочее. Тогда уж лучше взять Map<«Enum», Object>, чтобы хоть как-то обозначить границы и возможности.
Опять таки пример с билдером — всего лишь один из возможных путей решения. В реальном проекте кончено же, всё не просто проставляется. Те же цены, суммируются, сравниваются и прочее.
А вот из-за открытых полей с некоторыми коллегами можно и поссориться.pin2t
05.12.2019 21:55Билдеры в некоторых случаях действительно нужны. Например, StringBuilder действительно делает полезные вещи, он mutable и через это можно уменьшить расход памяти и циклов процессора. Но в данном случае билдер не делает ничего полезного, потому что объект просто инкапсулирует данные в конструкторе (что вообщем правильно и так и надо делать)
Насчет открытых полей, я как бы тоже не очень хорошо отношусь к открытым полям, но ведь бесполезные геттеры фактически и делают закрытые поля открытыми, геттеры которые ничего не делают мне кажется ещё хужеgeaker Автор
06.12.2019 00:24+1Тоже верно. Но с другой стороны, в этом и инкапсуляция. Кто-то вызывает геттер, не зная, как вырешивается значение. Таким образом, я могу позже добавить логику в сам геттер, не переписавая сами вызовы, коих к тому времени уже может быть много.
pin2t
06.12.2019 18:20В данном конкретном случае геттер ничего не вырешивает, а просто возвращает значение, фактически разрушая инкапсуляцию, внутренние данные объекта становятся доступны извне «как есть»
thelongrunsmoke
06.12.2019 11:35Не стоит пихать в конструкторы всё подряд. Пришла вам отрицательная цена, надо бы бросить какой-нибудь WrongPriceException, а делать это плохая идея. Java хоть и позволяет генерировать исключения в конструкторе, но на деле, вы получаете не инициализированный до конца объект который занимает место в памяти, и исключение которое может провалится туда, где оно не ожидается.
pin2t
06.12.2019 17:35Ну а в случае использования билдера и WrongPriceException вы получаете не инициализированный до конца билдер. И в чем разница.
thelongrunsmoke
06.12.2019 17:47В том, что билдер инициализируется конструктором по умолчанию, а потом заполняется данными, и каждый сеттер может производить валидацию и генерировать свои исключения.
user_man
05.12.2019 17:13Начинающий программист удивляется встрече с объективной реальностью:
Мне не нравится, что ни одна функция не написана в стиле ФП
Потом вот так:
не люблю тесты
Или вот так:
Испльзуя mock'и, я сумел протестировать код
И наконец вот такой перл:
С другой стороны, такой подход заставляет писать столько кода, что от него сразу хочется отказаться
В целом в статье видим предвзятый взгляд начинающего разработчика на слегка неидеальное решение другого, более опытного разработчика.
Собственно указание на ФП в самом начале уже прямым текстом говорит об отсутствии опыта работы с императивными подходами, иначе фразы типа «мне не нравится без ФП» просто не встречались бы. Но даже если забыть про ФП, то заявление «мне не нравится», не подкреплённое аргументами, выглядит как типичное недовольство не разбирающегося в проблеме, но считающего, что всё, что он видит на работе ему должно нравиться.
В тексте программы, конечно же, используется новомодный синтаксис, скрывающий от разработчика тип используемого объекта. Это всё привычно для разработчиков на JavaScript или на чём-то другом плохо типизированном, но в Java это выглядит страшно. Хотя, к сожалению, такую возможность всё же включили в какую-то из последних версий языка. Но тем не менее, привычка писать в таком стиле очередной раз показывает — перед нами самый что ни на есть начинающий разработчик на Java, скорее всего пришедший из мира редко используемых в промышленной разработке языков, что в свою очередь намекает на отсутствие опыта в промышленной разработке вообще.
Ну и собственно по смыслу проблемы. Автор видит простенькую неидеальность и пытается сделать код «совершенным». Это тоже типичная проблема новичка. И решает её новичок в таком виде, про который он сам говорит — «такой подход заставляет писать столько кода, что от него сразу хочется отказаться». Ну что-ж, полезная самокритика. Но вот результат всё же не впечатляет.
Почему результат не впечатляет? Очень просто — автор привык оптимизировать по сложно формализуемому критерию исключительно учебные кейсы для ФП, а потому автоматически применяет этот навык в более серьёзной разработке. В результате у него получается, как он сам отметил, нагромождение всего на свете.
Как исправить ситуацию? Всегда самый эффективный способ улучшения находится на уровень, а то и два, выше примитивов непосредственно кода. То есть алгоритм решения проблемы никак не зависит от любого языка программирования, но привычка новичков уделять внимание «красоте кода» (в их очень субъективном понимании) заставляет их строить то самое нагромождение, от которого сам же автор в конце концов плюётся.
Не надо переходить на уровень кода, когда ваш алгоритм глупый. Просто не надо.
Почему алгоритм глупый? Потому что автор даже не пытается понять, а что реально происходит в программе. Он видит только код и его субъективную «красоту». А что этот код делает?
Можно предположить, что объект типа Product заполняется данными. И точно так же можно предположить, что объект типа Product служит источником для заполнения каких-то дополнительных объектов. Это два архитектурно принципиально разных подхода. И решение усмотренной автором небольшой неидеальности должно в первую очередь зависеть именно от того, в каком направлении движутся данные. А на более высоком уровне всё зависит от того, зачем вообще эти данные куда-то движутся. В результате скорее всего весь рассматриваемый автором метод вообще не нужен, например потому, что итак уже содержащий данные объект типа Product (во втором варианте), можно направить в то место, где эти данные каким-то образом потребляются. Но повторюсь — автор не видит ситуацию «в большом», то есть вместо понимания ненужности условного дома, он старательно штукатурит его стены, что является самым главным признаком начинающего разработчика. Никакие зазубренные знания синтаксиса не помогут вам сделать глупость умной. Потому что глупость находится на несколько уровней выше в иерархии абстракций, по сравнению с синтаксисом языка. И вот это понимание затмевает зубрёжка синтаксиса, желание видеть очень условно понимаемую «красоту», основой для которой являются примеры из ФП, часто не имеющие к реальной жизни практически никакого отношения (да, всё те же сортировки, перестановки, операции над структурами). Погружаясь во все эти сортировки человек видит условную «красоту» низкоуровневых алгоритмов, а собственно зачем эти все алгоритмы применяются — ему совершенно по барабану. Ну и в результате имеем глупость.
Поэтому, дети (начинающие программисты) — зрите в корень! Не ведитесь на гламурный синтаксис и мнение функциональных программистов о «некрасивости» кода без функциональной шелухи. Думайте о главном. И даже больше — думайте, что такое главное. И может быть в итоге вы станете реально хорошими (полезными для проекта) разработчиками, а не поставщиками гламурной чешуи под рекламным называнием «красивый код».
epishman
05.12.2019 19:19Насколько я понимаю, сейчас в институтах сильно ФП форсят, поэтому и возникают такие казусы. Но ведь опыт преподать невозможно, его можно только настрогать самому. И да, как бы не хейтили ООП в разных статьях — это по прежнему основной стиль больших прикладных программ. Знаю случай, когда молодые красивые пытались в энтерпрайзе писать на Go, в итоге вернулись к Java, потому что наследование, женерики, исключения, и т.д.
geaker Автор
05.12.2019 19:20Спасибо за такой развёрнутый ответ.
Интересное, что у Вас сложилось именно такое мнение.
Наверное, мне надо было выражаться яснее.
Я не думаю, что кто-то из нас не сталкивался с императивным стилем программирования, т.к. именно с него обычно и начинается обучение.
Но Вы правы, не стоит слепо идти за каким-то стилем. Как я и сам уже сказал, главное, чтобы код работал.
Про тесты могу только уточнить, что я не писал, что не люблю тесты в целом. Лишь те, в которых идёт слишком сильная привязка на внутрености кода. Мне больше нравятся тесты наподобие «если я передаю А, то мне вернётся Б», нежелие те, в которых проверятся, а был ли вызван такой-то сервис, с помощью того же Mockito.verify (необходимость таких тестов я не обсуждаю).
К слову о Mockito: я имел ввиду, что с помощью этого прекрасного инструмента можно написать тесты для функций, основанных на интрефейсах, не реализируя сами интрефейсы.
Против императивного программирования или любого другого абсолютно ничего не имею.
Скорее всего просто разница во вкусах и предпочтениях заставила меня, заострить внимание на этом коде. Совершенного кода просто не бывает.user_man
06.12.2019 20:23+1>> Мне больше нравятся тесты наподобие «если я передаю А, то мне вернётся Б», нежели те, в которых проверяется, а был ли вызван такой-то сервис, с помощью того же Mockito.verify (необходимость таких тестов я не обсуждаю).
Тесты, как и вообще всё программирование, это инструмент. Может нравится молоток? С точки зрения эстетики, наверное, но оценивать его нужно с точки зрения способности забивать гвозди. И если гвозди забиваются нормально — молоток хороший. Здесь нет места эмоциям. Просто работа молотком. И всё. Хотя по началу (по молодости) из-за угла может показаться некий ореол романтичности, но это именно по началу.
Вообще, проверка «был ли вызван сервис» бывает разная. Например — нужно уведомлять внешнего клиента о произошедшем событии, тогда очевидно, что это важная часть функционала и вызов сервиса является критичным. Но бывает и по другому — внешний сервис просто даёт какую-то информацию, а потом, обработав эту информацию, процесс выдаёт какой-то свой результат. В таком случае проверка вызова сервиса бессмысленна, потому что нам важен результат, а не какие-то промежуточные действия. Если результат не сходится с ожиданиями — это критично, а был ли там в середине какой-то вызов — совершенно не важно, потому что архитектуру могут переделать буквально через месяц (и такое бывает) и все тесты «про вызов» пойдут псу под хвост. А вот если проверяли результат — переделывайте свою архитектуру сколько хотите, тест всё равно будет полезным.
SergeyEgorov
06.12.2019 11:20+1Мдауш...
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));
Что правда кто-то пишет такой код в реальности? В проектах, которые в проде обслуживают реальных потребителей?
epishman
Все равно проблема последовательности вызовов Вас рано или поздно догонит. Пустой объект — ok, а если данные нужно изменить? Мы строим дом — сначала стены, потом крыша, и никак иначе. ФП предлагает создавать новый дом при настиле кровли, но старый дом не висел в воздухе, он стоял на фундаменте, новый придется ставить туда заново, а там крепежные элементы, техпроцесс, и так далее. Если мы строим дом — это один и тот же дом, его можно прогонять через композицию функций, но это все равно мутабельный объект.
Carduelis
Как мне кажется, в таких кейсах с построением дома и как у автора (где допускаем высокий приоритет производительности), нужна явная очередь, хранимая внутри строящегося объекта, и проверки внутри функций.
Но в большинстве случаев, иммутабельность и чистые функции (и не важно, ООП у нас или нет), позволяют избежать многих непонятных и скрытых багов. Преждевременная оптимизация — ну, вы поняли. Нашли бутылочное горлышко — декомпозировали, мемоизировали, на крайний случай вынесли в явно-названную мутабельную функцию.
epishman
ФП это очень круто с точки зрения тестируемости и безопасности. Про оптимизацию не уверен, всеж самый быстрый код до сих пор на плюсах. А бывают задачи, когда без мутабельности никак, когда объекты огромные, привязаны цепями к базам данных и т.д. тут только имперaтивщина и циклы, потому что все остальное (включая передачу владения а ля Rust) — это дополнительный оверхед.
0xd34df00d
Тогда вполне можно написать с локальной мутабельностью.
И, кстати, если так написать, то на плюсах не получается быстрее всего. Я как раз на этой неделе поковырял задачку, где код на хаскеле, пусть и не совсем идиоматичный, оказался на двух доступных мне машинах существенно быстрее (20-40%) эквивалентного кода на плюсах (и на недоступной мне машине с процессором совсем другой архитектуры оказался ровно такой же производительности). Причин чего, кстати, я понять не могу, а дизассемблировать хаскель-код я не хочу.
И, кстати, без циклов, чистая хвостовая рекурсия.
Статейку наваять про это, что ли.
epishman
Эквивалентный код на плюсах — это неконкретно, слишком уж языки разные :)
0xd34df00d
Ну ок.
Х-ль (код уродливый, я предупреждал), плюсы.
Один знакомый, которому я это показывал, говорит, что ручной LICM для
s1[i]и разворачиваниеstd::minв его случае заметно ускорили плюсокод, но мне это воспроизвести не удалось.Сравнивал на i7 3930k и i5 8350U, в обоих случаях
g++ -O3 -march=nativeдля плюсов, gcc 9.2 (clang тоже пробовал, но он тут подчистую сливает gcc, что иронично, так как хаскель тут компилируется через llvm).epishman
Забавно, спасибо. Про успехи хаскеля слышал, но никогда на нем не писал, сохраню в закладки ))
Deosis
В плюсах расстояния хранятся в 64-хбитных переменных.
Вы тестировали функцию на строках длиннее 4-х миллиардов символов?
Надо проверить, что скорость не увеличится от перехода на 32 бита, если это возможно.
skymorp
В хаскеле раздрядность Int тоже 64, так что все честно.
P.S. по крайней мере у меня такая разрядность...
0xd34df00d
Да, разрядность
Intсоответствует битности машины.0xd34df00d
Нет, конечно, я бы не дождался окончания. 20 тысяч символов — порядка секунды, а временная сложность там квадратичная.
На моей машине она незначительно, но статистически значимо падает при замене на
uint32_tили даже наuint16_t.geaker Автор
ФП, ООП или что-то другое? В первую очередь я не задумывался о самом стиле, а именно о возможности, обезопасить код не комментарием, а как-то иначе.
В каждом стиле есть свои преимущества и недостатки
DarthVictor
То что это один и тот же мутабельный объект совсем не значит что у него на протяжении всей его жизни один и тот же тип. У реального дома со сменой состоянием меняется и его тип. Сначала это тип Котлован, потом Фундамент, потом ПервыйЭтажГотов. Причем в общем случае эти типы между собой никак не связаны. Тип Котлован вообще может не иметь общих методов с ТолькоЧерновойРемонтДом.
Просто многие современные языки программирования не позволяют менять тип объекта после создания кроме как ручным приведением между типами с общим родительским типом, поэтому объект проще пересоздать. Да и нагляднее это. Но это особенность языков программирования и сложности выводов типов в компиляторах. Тот же TypeScript умеет в вывод конкретного типа из объединения на основе значения одного из полей. Но опять же может быть нагляднее просто пересоздать объект.
epishman
Менять тип в рантайме — это рушит весь смысл типизации. В случае с домом оптимально все ж иметь один мутабельный объект, внутри может быть композиция, наружу фасады, но никаких приведений типов.
DarthVictor
Я не очень понял о чем Вы, возможно что Вы и правы.
По-моему же, менять тип в рантайме плохо вот по какой причине.
Продолжая пример стройки, у Вас есть тип Дом, равный алгебраической сумме всех типов состояний этого дома.
Дом = Котлован | Фундамент | ПервыйЭтажГотов | ... | ТолькоЧерновойРемонтДом | ЧистовойРемонтДом;Код, работающий только с типом Фундамент, не скомпилируется с типом Котлован, пока вы не вызовите на своем Котловане метод залитьФундамент.
Я вижу проблему в том после вызова заливкиКотлована, где-то осталась ссылка на дом, когда он еще был котлованом, и кто-то может попробовать вызвать метод.рытьЛопатой у залитого фундамента. Но строго говоря если бы у нас на протяжении всего процесса был бы один тип Дом, вам все равно бы пришлось проверять в методе рытьЛопатой текущий статус дома.
То есть менять тип в рантайме переменной плохо, потому же почему и просто лишний раз ее менять плохо — кто-то мог с этой переменной работать и не закончить.
0xd34df00d
Все хорошо, но зачем здесь тип
Дом?Vlad800
EvgeniiR
Спасибо, положу себе в копилку самых "весёлых" определений ООП. Интересно, как вы себе другие парадигмы тогда представляете. Без ООП пользователям сразу write-доступ в базу с платежами даётся?
P.s. На самом деле грустно это. Сегодня я уже упоминал buzzword-driven подход.
Vlad800
Как будто других парадигм много…
epishman
В JS когда вы меняете тип переменной в рантайме, сразу слетает вся JIT-оптимизация, и производительность падает в разы. Типы нужны чтобы их компилятор проверял, и оптимизировал аллокации памяти. Если Вы так хотите обезопаситься от установки кровли на голый котлован — можно сделать прокси-объект, проверяя все комбинации параметров в одном месте.
DarthVictor
В JS в рантайме значительной части типов вообще нет. Остаются только классы.
Я все-таки очень вырожденный пример дал. Во фронтенде например такая сложная обработка состояния, если вообще потребуется будет внутри какого-нибудь Redux/Mobx/Vuex еще чего-то, которые и будут выступать прокси объектом. При этом входной код будет только получать на вход объект с как минимум одинм обязательным поле.состояниеСтроительстваДома и кучей возможных полей. Подписанные на этот объект компоненты будут просто получать уведомление о том, что объект поменялся. Как именно он должен поменяться будет определяться менеджером состояний, который и будет выступать фасадом.
michael_vostrikov
Типизация не означает проверки только во время компиляции и не означает, что в каждом конкретном месте должен использоваться только один тип. Типизация это проверки типов. Даже если тип меняется в рантайме, с типизацией он будет оставаться в пределах допустимых типов.