Нашли или выдавили из себя код, который нельзя назвать нормальным, на который без улыбки не взглянешь? Не торопитесь его удалять или рефакторить, — запостите его на говнокод.ру, посмеёмся вместе!
+114
private List<PalletePath> killDupes(List<PalletePath> pathesNew) {
List<PalletePath> noDupes = new ArrayList<PalletePath>();
for (PalletePath tp : pathesNew) {
if (!noDupes.contains(tp)) {
noDupes.add(tp);
}
}
return noDupes;
}Set? Не, не слышали.
Запостил: 
someone,
05 Марта 2013
Follow us!
по семантике полностью аналогично приведённой писанине, при этом не требуя квадратичного времени.
А этот метод настолько часто вызывается и оперирует такими объемами данных, что квадратичное время становится критичным?
Простой пример - singleton. Правильная реализация - DCLP (java 5) или instance holder. Но и
также является правильной реализацией в однопоточном приложении.
все решает контекст
По производительности такой синглтон не особо быстрее потокобезопасного. По длине кода - ну чуть-чуть короче. А вот проблем тем, кто будет его поддерживать, он всяко добавит. У меня один вопрос: нахуя зачем?
> Правильная реализация - DCLP (java 5)
DCLP это срань, которая работает по счастливой случайности даже с новой моделью памяти java 5. От авторов спеки: There exist a number of common but dubious coding idioms, such as the double-checked locking idiom, that are proposed to allow threads to communicate without synchronization. Almost all such idioms are invalid under the existing semantics, and are expected to remain invalid under the proposed semantics.
> видать много серверного кода писать приходится :)
Любая прога, использующая swing, не однопоточна - там минимум 2 треда - стартовый и EDT. На том же андроиде в главном треде невозможно работать с сетью (кидает NetworkInMainThreadException). Так что даже совсем не серверные проги бывают многопоточными...
Код - это декларация намерений. Я всегда читаю чужой код с мыслью, что его писал умный человек. Т. о. любая строчка была осмысленной и обоснованной в тот момент когда она писалась. И если, возвращаясь, к singleton'у, я вижу реализацию не потокобезопасную, то я делаю вывод, что эта часть кода не многопоточна, но ни в коей мере не делаю вывод, что автор идиот. Ибо если тут написано для одного потока, то помимо этого несчастного singleton'а наверняка есть еще много кода, который также однопоточен. Непотокобезопасный singleton - это один способов из декларации того, что автор не предполагал использование кода в многопоточном окружении.
Возвращаемся к исходному коду.
Если бы такой код был у кого-нибудь из моих коллег я бы сделал следующие выводы:
1. Быстродействие здесь не критично (маленькие объемы + редко вызывается)
2. Коллега запамятовал про LinkedHashSet
т. к. я на 100% уверен, что если быстродействие было бы критичным (но про LinkedHashSet коллега не вспомнил) то код выглядел бы как-нибудь так
Поэтому я не сторонник говорить правильный код или нет не имея контекста.
ПС. Попытка писать сразу универсальный код - это одна из главных проблем умных программистов. Желание писать сразу все универсально приносит больше проблем, чем профита.
Это совсем другое дело
> я вижу реализацию не потокобезопасную
Смотреть в реализацию - большое зло, часто, к сожалению, необходимое. К тому же, прекрасный и простой синглетон в жабе легко реализуется через enum.
> Быстродействие здесь не критично
Нормальное быстрое решение реализуется в три строчки, плохое и потенциально медленное - в девять. Не люблю я лишний код, не люблю.
С резюме согласен, к сожалению, желание предусмотреть все варианты и "писать с рассчётом на будущее" далеко не всегда себя оправдывает, редко можно угадать, что именно потребуется.
Вроде бы есть простое объяснение: где можно малыми силами сделать "правильно" делаем сразу правильно, а где это требует телодвижений и увеличения кода (неоправданно) - там не делаем. Основной посыл получается такой: когда будем доделывать меньше придется переделывать.
Сам так делал, сейчас от этого стараюсь максимально отойти. Ибо такой код потом может подкинуть проблем. Например, другому человеку поручат в нем разобраться, или сам через полгодика в него залезешь. По такому сложно понять о чем думал человек, код выглядит как взрыв на макаронной фабрике, не прослеживается четкой нити: здесь так, а здесь вот так, а здесь вообще иначе.
Логично написанный код может рассказать порой намного более javadoc'ов и комментариев
2. Смотреть приходится при командной разработке проекта, без этого уж точно никак
по моему опыту, код делится на 2 типа:
1. код обязательно переписывается под постоянно меняющиеся требования, ввиду чего невозможно писать код на века
2. код в силу трудностей сопровождения долгое время находится в ореоле мистики и оберегается, пока "работает же", опять же до критической массы багов
вывод: нет смысла писать универсальное решение, оно вряд ли себя оправдает. есть смысл писать решение, которое также легко пишется, как и читается, без особых премудростей.
Зачем же все? ;) Тем, которые гарантированно крутятся в своем потоке и не выходят за его пределы, синхронизация нахер не сдалась. А вот публичный синглтон, как и любую другую глобально-публичную переменную лучше лишний раз синхронизнуть чем потом об этом жалеть.
> Непотокобезопасный singleton - это один способов из декларации того, что автор не предполагал использование кода в многопоточном окружении.
Написал публичный однопоточный синглтон - опиши в комментарии/документации к нему какие требования и мысли привели к такому поступку, поясни читателю в каких именно случаях его использование безопасно. Например "Этот синглтон в свое время являлся bottleneck'ом при отрисовке сцены, поэтому было принято решение сделать убрать синхронизацию и использовать его только из event dispatch thread.".
> Желание писать сразу все универсально приносит больше проблем, чем профита.
Всё - да. Места, в которых потом явно вылезут гейзенбаги, костыли и прочее говно - нет.
2. javadocи быстро устаревают
3. далеко не всегда можно знать где вылезут костыли
По чьей вине? ;) Или они сами умирают от старости?
> далеко не всегда можно знать где вылезут костыли
Полностью согласен, но однопоточный синглтон без веской причины - отличный зародыш гейзенбага.
2. Однопоточный синглтон - это такой большой красный флаг "Эй! Будь аккуратнее! Тут все надо проверить!". Согласитесь: если человек точно занет, что в будущем будем многопоточность так код не напишет. Значит он был уверен на 100% и даже в страшных снах не видел возможности многопоточного использования. С другой стороны, если он знал, что многопоточность будет внедряться, то и весь код будет написан таким образом, чтобы внедрение заняло минимум времени и было предельно простым.
А если он ни во сне ни на яву не видел потоков, и просто сляпал синглтон из того что знал?
> Однопоточный синглтон - это такой большой красный флаг "Эй! Будь аккуратнее! Тут все надо проверить!"
Это если есть исходники. А если либа досталась в бинарном виде? Декомпилировать и изучать? Или предполагать худшее и добавлять к этому неведомому синглтону свою синхронизацию?
Если код изначально был многопоточным, а программист сляпал такое - это уже другой разговор. Это действительно бомба.
Если вы выпускаете библиотеку и отдаете ее людям - документация must have.
Так бывает далеко не всегда.
У говнкокодеров - да. Поэтому лучше всего делать, как опенсорсники, т.е. не писать доки вообще.
Работа с сетью - не показатель. В тех же мобильных приложениях работа сетью чаще всего сводится к цепочке:
UI: Показал Loading... и заблокировал пользователя
NotUI: отправил запрос на сервер
NotUI: получил ответ
NotUI: произвел преобразования ответа в то что необходимо (хорошо, если это еще сделали не в UI потоке)
UI:Отобразил на экране и разблокировал пользователя.
Я это не считаю за многопоточность, это просто необходимый минимум.
А вот процессор и компилятор, внезапно, считают...
Соответственно человек, пишущий свинговые приложения - гуру многопоточности. И в резюме он может смело писать - знаю многопоточность как свои 5 пальцев. Это как у уборщика в Шереметьево спрашивают: "Почему вы не смените работу?", а в ответ: "Да чтобы я бросил авиацию?! НИКОГДА!!!"
П. С. Не стоит воспринимать слова лишь с формальной точки зрения. Иначе любой диалог будет сводиться к абсурду.
Если 2 потока используют общую переменную - это многопоточность. Хоть с формальной хоть с неформальной точки зрения. И если библиотека делает всю синхронизацию за вас (как например это делает андроидский AsyncTask), то это не повод называть такое приложение однопоточным.
есть же случаи, когда двум потокам необязательно иметь актуальные данные.
> есть же случаи, когда двум потокам необязательно иметь актуальные данные.
Есть. Но как правило тогда эти данные можно упрятать куда-нибудь в приват или в модуль. По крайней мере в сишке я старался так делать.
У меня было все проще. Приложение посылает запрос на сервер "дай мне последние новости начиная с 7 марта" в ответ получает, например, json. И после этого в UI потоке (ибо так проще) конвертит этот json в объекты и обновляет интерфейс.
То что здесь есть потоки - бесспорно. Но это их тривиальнейшее использование у меня язык не поворачивается назвать многоточностью. Хотя по всем законам это многопоточность.
Вот если вы параллелите какие-либо вычисления, если у вас висят фоновые задачи, которые меняют данные и интерфейс вашего приложения в режиме реального времени их отображает - вот это я подразумеваю, когда говорю о многопоточности в этой ветке.
Здесь мы видим 2 межпоточных передачи данных - на первой парамеры запроса ушли в worker, а на второй полученный json передали в UI поток, в котором его будут парсить и показывать. Да, не спорю, удобная либа скрыла от нас эти детали, и мы о них лишний раз не задумываемся...
Но забывать о том, что это именно многопоточный сценарий все же не стоит. Здесь есть как минимум 2 разделяемых объекта - те самые запрос и ответ. А если воркер лезет к какому-нибудь не локальному объекту, например за куками, то добавятся и еще... И здесь шанс наступить на грабли, если забыть о многопоточности, ничуть не меньше, чем при параллельных вычислениях ;)
Скажите честно, если бы вы поручили новичку реализовать данный сценарий, то сказали бы ему "не бойся, здесь нет никакой многопоточности", или все-таки перечислилили бы ему элементарные правила, о том что нельзя делать в таких ситуациях?
Задача воркера послать запрос передав String, подождать ответ и в чистом виде передать вернувшийся String в колбэк. Никуда вовне worker лазить не будет, никакие объекты сам обновлять не будет. Он только ждет.
Мой сценарий по силам новичку.
Да, со стрингом, как и с любым другим иммутабельным объектом очень трудно что-нибудь забаговать, тут мне нечего возразить.
> Мой сценарий по силам новичку.
Который имеет базовые представления о многопоточности. Те самые "никуда вовне worker лазить не будет", "никакие объекты сам обновлять не будет" вы же не от балды придумали, а составили на основе своих знаний о потоках ;)
Сетевое взаимодействие часто прячется за фреймворком, который дает UI callback. Т. е. вроде как все работает из коробки, пусть и не оптимально. И многим этого хватает. Тут о многопоточности в моем понимании говорить не приходится.
Когда же люди сами реализуют такие фрейморки или пишут воркеров суть которых более сложная, чем просто попроси/подожди/отдай - вот тут уже можно о чем-то говорить.
ПС. Кстати, если в моем конкретном примере кто-то из воркера решит куда-то слазить - я решу, что у него не все впорядке с головой :)
точно так же когда-то написал.