Добро пожаловать в форум, Guest  >>   Войти | Регистрация | Поиск | Правила | В избранное | Подписаться
Все форумы / Java Новый топик    Ответить
Топик располагается на нескольких страницах: [1] 2 3 4   вперед  Ctrl      все
 Новые веяния  [new]
xerxf
Member

Откуда:
Сообщений: 128
Последнее время делаю довольно много code review и замечаю такую вещь - пошло прямо таки какое то повальное увлечение переусложнением кода. Что это - не понимание сути и попытка впихнуть везде, где только можно свежеполученные знания или реальная необходимость, которой я никак не могу проникнуться?
Как пример:
Есть некая функция. которая что то там делает, но основная логика скрыта в вызываемой библиотечной функции. Эта библиотечная функция синхронизированна. И возвращает список. причём эта библиотечная функция в классе-синглтоне.
По логике программы эта функция должна вызываться в цикле несколько раз с разными параметрами. Программист оборачивает функцию в Future, внутри результаты работы библиотечной функции пишет в потокобезопасную коллекцию(причём над коллекцией он тоже сделал обёртку-синглтон), и все эти фьчерсы засылает в CompletableFuture.allOf. А по whenComplete выбирает данные из потокобезопасной коллекции и что то с ними делает. Вопрос - а зачем так сложно? всё равно же всё споткнётся на синхронизированном методе ( а там кроме него по сути и нет ничего) и будет выполняться фактически последовательно, без всякого парралелилизма?
Или ещё - функция возвращает строку. Эта строка передаётся в другую функцию. Другая функция этой строке что то там ищет. Всё. Зачем первая функция строку оборачивает в Option ? Не, я всё понимаю - функциональщина, защита от null и всё такое.. Но как бы любые лишние телодижения - это лишние тормоза и каждая лишняя строчка - это потенциальный источник проблем (все мы человеки - чем больше напишешь, тем больше вероятость ошибки) зачем это всё? особенно если первая функция гарантированно возвращает не null
Или ещё - есть некие действия, которые нужно делать в отдельном потоке. Данные для них приходят периодичеки. Для каждой приходящей порции данных создаётся свой поток, в нём по каким то условиям стартуют ещё потоки, там создаются какие то фьючерсы, а потом это всё долго и мучительно синхронизируется.... Но там всё что надо - это чтобы данные обрабатывались НЕ в основном потоке. И абсолютно наплевать, в одном потоке они обработаются или в 10.. После того, как все эти сложности повыкидывал - необходимость в синхронизации вообще отпала. и работать всё стало в разы стабильнее. Опять таки - что это было ? непонимание потоков автором или я опять что то не понимаю в современном стиле программирования на java ? А то я когда то на плюсах начинал - может такое отношение ко всем этим наворотам это просто издержки моего прошлого ?
16 июл 20, 10:35    [22168384]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Андрей Панфилов
Member

Откуда: Москва > Melbourne
Сообщений: 3625
xerxf,

чет у вас в достаточно тяжелой форме все проходит. Я только замечал повальное стремление заменить все подряд на java.util.function.*, чтобы потом невозможно было найти реализацию. А так ваш коллега скорее всего имеет опыт разработки на JS - там только так и пишут.

Сообщение было отредактировано: 16 июл 20, 11:03
16 июл 20, 11:05    [22168405]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
xerxf
Member

Откуда:
Сообщений: 128
Андрей Панфилов,

Да, реализацию найти тоже проблематично. Обёртка над обёрткой, а там функциональщина..
Просто ыглядит красиво, солидно, модно, стильно, молодёжно. Я бы тупо написал вызов функции в цикле с накоплением результата. А там накручено - выглядит уж больно солидно
Я писал на js - стиль может и похож, но не помню бесмысленного на мой взгляд переусложнения.
Я пытаюсь понять - оно бессмысленно только на мой взгляд или одно из двух...

Сообщение было отредактировано: 16 июл 20, 11:17
16 июл 20, 11:11    [22168409]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Alexey Tomin
Member

Откуда: Самара
Сообщений: 1898
xerxf
Последнее время делаю довольно много code review и замечаю такую вещь - пошло прямо таки какое то повальное увлечение переусложнением кода.


Как начал программировать не один так и замечаю.
Не от языка это зависит. Да и не новое это.
Просто умение писать кратко и понятно- это нечастое и очень ценное умение.
16 июл 20, 11:45    [22168450]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
Это пока всего лишь твои слова, которым мы можем либо доверять либо нет. Тут говорится 50\50, не видя кода сложно сказать прав ты или нет, всегда есть вероятность что ты реально чего-то не понимаешь. Но для этого и есть code review - чтобы задавать вопросы а почему так а не эдак? Странным выглядит твой вопрос тут а не в PR.
Что касается вопросов
1) А вдруг реализация библиотечной функции изменится, либо вы ее вообще выкинете и перепишете на свое, забыв про синхронизацию?
2) А может все-таки там нулл возможен? Либо вторая функция используется в нескольких местах, и в каком то месте туда реально может придти опшионал?
3) Ну тут вообще нифига непонятно, ты тесты многопоточные написал что реализация корректна до твоих изменений и после?
16 июл 20, 12:11    [22168483]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
chpasha
Member

Откуда:
Сообщений: 9552
забыл ник
Это пока всего лишь твои слова, которым мы можем либо доверять либо нет. Тут говорится 50\50

+1
16 июл 20, 12:46    [22168522]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
andreykaT
Member

Откуда: Finland
Сообщений: 2955
xerxf
Андрей Панфилов,

Да, реализацию найти тоже проблематично. Обёртка над обёрткой, а там функциональщина..
Просто ыглядит красиво, солидно, модно, стильно, молодёжно. Я бы тупо написал вызов функции в цикле с накоплением результата. А там накручено - выглядит уж больно солидно
Я писал на js - стиль может и похож, но не помню бесмысленного на мой взгляд переусложнения.
Я пытаюсь понять - оно бессмысленно только на мой взгляд или одно из двух...

вызов функции в цикле с накоплением. какой то прошлый век )) функциональщина и проще и понятнее ведь? в разумных пределах разумеется.
16 июл 20, 13:18    [22168553]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
andreykaT
Member

Откуда: Finland
Сообщений: 2955
хотя на ней все шизанулись. смотрю ваки скалы. всем подавай катс и зио. причем по зио опыт 3 года минимум. Типа если ты не пишешь на зио - ты ничтожество. и да. 3 года. не меньше. с учетом того что этому "фреймворку" год от силы. всем подавай самое новое.
16 июл 20, 13:20    [22168559]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Leonid Kudryavtsev
Member

Откуда:
Сообщений: 8762
забыл ник

А вдруг реализация библиотечной функции изменится, либо вы ее вообще выкинете и перепишете на свое, забыв про синхронизацию?

Аргумент просто пи....ц какой-то
Если бы, да кабы
А если в функции вызовут System.exec( "halt"); ? Что делать? Все пропало!

Чувствую, что CompletableFuture там недостаточно. Код говно. Обязательно нужно не забыть обернуть еще в HaltFuture, OutOfMemoryFuture и в другие такие же полезные классы. Только этим путем проект еще можно спасти!
16 июл 20, 13:22    [22168561]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
nnalogovik
Member [заблокирован]

Откуда:
Сообщений: 160
забыл ник

1) А вдруг реализация библиотечной функции изменится, либо вы ее вообще выкинете и перепишете на свое, забыв про синхронизацию?


А вдруг в проект придет сумасшедший блатной кодер - который удалит весь код
16 июл 20, 13:47    [22168585]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
chpasha
Member

Откуда:
Сообщений: 9552
и пошел спор о сферических конях в вакууме. пусть ТС запостит код, тогда и поговорим о вариантах
16 июл 20, 14:00    [22168608]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Leonid Kudryavtsev
Member

Откуда:
Сообщений: 8762
Появились конструкции, которые могут помочь сделать код ассинхронным, многопоточным, быстрым и более устойчивым к ошибкам.
Но автоматически, сам собой, код все равно таким не станет. Для ассинхроннсоти все равно нужно продумывать, как осмысленно разруливать коллезии. Для ускорения все равно нужно профайлить и разбираться где и почему затыки.

А в реальной жизни происходит подмена понятий, люди начинают считать, что если они запихнут эти конструкции куда нужно и куда не нужно, то код станет таким сам собой, волшебным образом.
Если у нас в коде CompletableFuture - значит, sic!, код ассинхронный, многопоточный и быстрый по определение!
Если у нас в коде Option - значит ошибок у нас нет! мы же все пишем в соответствии с мировыми практиками.

Если весь проект набит CompletableFuture, Option, но почему-то затыкается на 600 инсертах в секунду, то это PostgreSQL виноват ))) и нужно добавить ака, кафку и редиса для большей асинхронности и скорости )))
/тут смайлики! все совпадения с какими нибудь темами на форуме совершенно случайны/
16 июл 20, 14:52    [22168655]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
Leonid Kudryavtsev
забыл ник

А вдруг реализация библиотечной функции изменится, либо вы ее вообще выкинете и перепишете на свое, забыв про синхронизацию?

Аргумент просто пи....ц какой-то
Если бы, да кабы
А если в функции вызовут System.exec( "halt"); ? Что делать? Все пропало!

Чувствую, что CompletableFuture там недостаточно. Код говно. Обязательно нужно не забыть обернуть еще в HaltFuture, OutOfMemoryFuture и в другие такие же полезные классы. Только этим путем проект еще можно спасти!

Аргумент в том, что мы не знаем код, что не мешает делать выводы о праивльности\неправильности. Я конечно не против ваших фантазий, только при чем они к тому что я написал?
16 июл 20, 15:33    [22168681]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
nnalogovik
забыл ник

1) А вдруг реализация библиотечной функции изменится, либо вы ее вообще выкинете и перепишете на свое, забыв про синхронизацию?


А вдруг в проект придет сумасшедший блатной кодер - который удалит весь код

Складывается впечатление что я должен обьяснить код, который не писал. Наверное ты видел тот код?
16 июл 20, 15:35    [22168682]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
Leonid Kudryavtsev
Появились конструкции, которые могут помочь сделать код ассинхронным, многопоточным, быстрым и более устойчивым к ошибкам.
Но автоматически, сам собой, код все равно таким не станет. Для ассинхроннсоти все равно нужно продумывать, как осмысленно разруливать коллезии. Для ускорения все равно нужно профайлить и разбираться где и почему затыки.

А в реальной жизни происходит подмена понятий, люди начинают считать, что если они запихнут эти конструкции куда нужно и куда не нужно, то код станет таким сам собой, волшебным образом.
Если у нас в коде CompletableFuture - значит, sic!, код ассинхронный, многопоточный и быстрый по определение!
Если у нас в коде Option - значит ошибок у нас нет! мы же все пишем в соответствии с мировыми практиками.

Если весь проект набит CompletableFuture, Option, но почему-то затыкается на 600 инсертах в секунду, то это PostgreSQL виноват ))) и нужно добавить ака, кафку и редиса для большей асинхронности и скорости )))
/тут смайлики! все совпадения с какими нибудь темами на форуме совершенно случайны/

Это все правильно. Вопрос в том, какая ситуация соотвествует действительности
1) Все написано правильно, просто ТС в силу отсутствия знаний не понимает почему написано именно так и делает поспешные выводы
2) Все действительно плохо и код ужасный.
Не видя кода отличить 1 от 2 не имеется практической возможности.
Можно конечно дальше сотрясать форум возмущением, только это имеет мало смысла
16 июл 20, 15:39    [22168683]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Leonid Kudryavtsev
Member

Откуда:
Сообщений: 8762
Даже имея код, делать какие-то выводы нельзя

В команде должен быть обшепринятый стиль работы более-менее удобный, понятный и принятый всеми (или общепринятый пофигизм, что тоже является стилем)

С одной стороны:
Если кто-то новый пришел в команду и еще не вписался в данный стиль - на коде review его должны поправить
Если он так и не смог вписаться, а его коллегам такой стиль удобен - он будет создавать проблемы окружающим, рано или поздно его пошлют нафиг )))

С другой стороны:
Если правила в данной компании не устраивают, будь это зарплата, качество кофе и печенюшек в офисе, дресс код или идиотские требования к коду - нужно искать новое место работы
Можно попытаться быть д'Артаньяном и что-то менять в мире вокруг себя, можно тихо смириться и попытаться быть как все, можно поискать другое место.

IMHO & AFAIK
16 июл 20, 15:54    [22168692]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
Zzz79
Member

Откуда:
Сообщений: 569
на счет этих Completeble фьюч согласен - везде напичкано
на код смотришь ничего не понятно - по полдня тратишь на поиски правды)
16 июл 20, 16:25    [22168722]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
Zzz79
на счет этих Completeble фьюч согласен - везде напичкано
на код смотришь ничего не понятно - по полдня тратишь на поиски правды)

Ну это эпичный пример неуклюжей попытки пропихнуть функциональный концепт из скалы в java, не понимая что лучше чем портировать часть решения - не портировать ничего. С опшинал примерно тоже самое.
А теперь попробую рассказать откуда у этого все ноги растут. Все эти Completable Future - это по сути колббаки или continuation. В скале и ФП есть такое понятие как монада. И вот там Future образует монаду(контекст continuation) которым легко и удобно пользоваться, который по сути всю асинхронность и сложность коллбэков превращает в зрительно императивный код.
val getEmployee(id: Int) = Future(DB.load)
val getDepartment(emp: Employee) = Future(Db.load)
val getBonus(dep: department) = Future(DB.load)

val result: Future[Bonus] = for { // тут мы как бы входим в контекст Future, то есть убираем этот враппер
  e <- getEmployee(1) -- тут работаем с энтити типами, никаких future
  d <- getDepartment(e)
  b <- getBonus(d)
} yield b //Обратно запаковываем во future

Тут все элегантно и красиво в отличие от java-ского
future.thenAsync().orElseThrow бла бла
16 июл 20, 16:34    [22168725]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
nnalogovik
Member [заблокирован]

Откуда:
Сообщений: 160
забыл ник
nnalogovik
пропущено...


А вдруг в проект придет сумасшедший блатной кодер - который удалит весь код

Складывается впечатление что я должен обьяснить код, который не писал. Наверное ты видел тот код?


Да не меня просто всегда впечатляли придирки к коду типа


"А что если придет джун - и не поймет тут эту функцию, напишет вот так и тогда все ляжет"
16 июл 20, 18:49    [22168799]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
nnalogovik
забыл ник
пропущено...

Складывается впечатление что я должен обьяснить код, который не писал. Наверное ты видел тот код?


Да не меня просто всегда впечатляли придирки к коду типа


"А что если придет джун - и не поймет тут эту функцию, напишет вот так и тогда все ляжет"


Вообще это цель любого API - чтобы тот кто будет использовать его, с наименьшей вероятностью допустил ошибку. Для этого на разных уровнях абстракции возможно поддерживать системные инварианты. Как например, что ни при каких раскладах система должна быть потокобезопасной. Кто знает как там было - может система была кастомная с кучей кода, а потом появилась либа которая делает тоже самое, только синхронизация у нее внутри, ну вот старый код выкинули а синхронизацию на всякий оставили(вдруг появится еще какая либа еще) и т.д и т.п. Так что да, аргумент вдруг придет джун он иногда имеет смысл
16 июл 20, 18:57    [22168801]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
chpasha
Member

Откуда:
Сообщений: 9552
как у нас: "а вдруг придет кто-то без знания HQL, давайте писать все запросы в Hibernate на Criteria-API". Никто без знания SQL/HQL так пока и не пришел, а ежики колются и пишут годами во всех проектах строго на Criteria-API :(
16 июл 20, 19:07    [22168806]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
забыл ник
Member

Откуда:
Сообщений: 3370
chpasha
как у нас: "а вдруг придет кто-то без знания HQL, давайте писать все запросы в Hibernate на Criteria-API". Никто без знания SQL/HQL так пока и не пришел, а ежики колются и пишут годами во всех проектах строго на Criteria-API :(

Ну тут надо все взвешивать. Если бизнес выиграет от переписывания(так как снизится цена поддержки и увеличится быстрота написания кода) и риск небольшой - то надо планировать таск и делать. Другое дело, что если риск того что система еб..тся и фирма потеряет миллионы долларов хотя бы чуть-чуть больше нуля - то нет и не может быть никакого разговора о переписывании.
16 июл 20, 19:19    [22168812]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
nnalogovik
Member [заблокирован]

Откуда:
Сообщений: 160
забыл ник
nnalogovik
пропущено...


Да не меня просто всегда впечатляли придирки к коду типа


"А что если придет джун - и не поймет тут эту функцию, напишет вот так и тогда все ляжет"


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



Все это от лукавого и ну очень натянуто

Джун если тупой - он и в самом хорошем API грабли найдет и на них наступит

А если умный - и с проблемным разберется


Не нужно на джуна перекладывать свои страхи и придирки
16 июл 20, 19:50    [22168828]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
andreykaT
Member

Откуда: Finland
Сообщений: 2955
chpasha
как у нас: "а вдруг придет кто-то без знания HQL, давайте писать все запросы в Hibernate на Criteria-API". Никто без знания SQL/HQL так пока и не пришел, а ежики колются и пишут годами во всех проектах строго на Criteria-API :(

Имхо критериа апи гораздо сложнее и к чтению и к пониманию нежели хкл или жпкл. Те хоть на скуль издали одним глазом похожи.
16 июл 20, 23:31    [22168905]     Ответить | Цитировать Сообщить модератору
 Re: Новые веяния  [new]
andreykaT
Member

Откуда: Finland
Сообщений: 2955
забыл ник
Zzz79
на счет этих Completeble фьюч согласен - везде напичкано
на код смотришь ничего не понятно - по полдня тратишь на поиски правды)

Ну это эпичный пример неуклюжей попытки пропихнуть функциональный концепт из скалы в java, не понимая что лучше чем портировать часть решения - не портировать ничего. С опшинал примерно тоже самое.
А теперь попробую рассказать откуда у этого все ноги растут. Все эти Completable Future - это по сути колббаки или continuation. В скале и ФП есть такое понятие как монада. И вот там Future образует монаду(контекст continuation) которым легко и удобно пользоваться, который по сути всю асинхронность и сложность коллбэков превращает в зрительно императивный код.
val getEmployee(id: Int) = Future(DB.load)
val getDepartment(emp: Employee) = Future(Db.load)
val getBonus(dep: department) = Future(DB.load)

val result: Future[Bonus] = for { // тут мы как бы входим в контекст Future, то есть убираем этот враппер
  e <- getEmployee(1) -- тут работаем с энтити типами, никаких future
  d <- getDepartment(e)
  b <- getBonus(d)
} yield b //Обратно запаковываем во future

Тут все элегантно и красиво в отличие от java-ского
future.thenAsync().orElseThrow бла бла

В скалке так же можешь писать мапами флатмапами. Мало будет отличаться от джавы. Но джава да уже морально устарела. Насчет читаемости форкомпрехеншена это тоже имхо субъективное мироощущение думая что они более элегантные.
16 июл 20, 23:33    [22168906]     Ответить | Цитировать Сообщить модератору
Топик располагается на нескольких страницах: [1] 2 3 4   вперед  Ctrl      все
Все форумы / Java Ответить