- 01
- 02
- 03
- 04
- 05
- 06
- 07
- 08
- 09
- 10
- 11
- 12
- 13
public int GetModuleId(int userId)
{
return moduleIdGet(userId);
}
protected int moduleIdGet(int userId)
{
int moduleId;
// calculate moduleId
// ...
return moduleId;
}
Нашли или выдавили из себя код, который нельзя назвать нормальным, на который без улыбки не взглянешь? Не торопитесь его удалять или рефакторить, — запостите его на говнокод.ру, посмеёмся вместе!
+136
public int GetModuleId(int userId)
{
return moduleIdGet(userId);
}
protected int moduleIdGet(int userId)
{
int moduleId;
// calculate moduleId
// ...
return moduleId;
}
Дал открытый доступ, но в то же время как бы сохранил защищённый.
А вот следующий ход - уже говно. Имхо, достаточно было поменять протектед на паблик, а не городить костыль со вторым методом...
А сужать то зачем? Если другие девелы пользуются, значит им это нужно. Значит код полезный.
Единственные причины которые знаю это повыпендриватся и/или кому-то падлу сделать. И это ненормально.
Ну вот дошло до разраба, что сдуру выставил что-то лишнее в паблик. Человеку свойственно ошибаться. Если это прога, а не либа - вполне можно загрепать вызовы и сузить, пока на эту фичу не сильно завязались.
> Если другие девелы пользуются
То поздно пить боржоми, разве что объявить как deprecated ;( О чем я говорил: расширить - да запросто, была бы веская причина, сузить - 7 кругов ада, и 5 лет поддержки.
В libpng (вроде бы в ней) сколько лет пытались переделать сдуру выставленые напоказ поля структуры на геттеры... Как бы не больше 5 лет... И один хрен народ опомнился только тогда, когда эти поля "внезапно" пропали.
> То поздно пить боржоми, разве что объявить как deprecated
Как у нас на проекте: в одной либе есть метод deprecated который уже как 5й год депрекэйтед. А толку, если другого способа доступится к функциональности так никто и не придумал.
Или как в одной либе хэш функция для генерации номеров. С самого начала сделали внутренней, что бы типа а не хер. 11 лет спустя, в проекте есть как минимум пять копий оного хэша. Хотел поменять на более эффективный - а мля обломись.
> В libpng (вроде бы в ней) сколько лет пытались переделать сдуру выставленые напоказ поля структуры на геттеры...
Это разрабы как-то по GNOME-овски поступли. Чисто технически, такое как раз решается просто: если ломаешь публичный интерфейс, так ломай его полностью, и переводи в новый нэймспэйс. Старый интерфейс просто делается как враппер поверху нового.
Ну, это уже неграмотное перепроектирование. Зочэм тогда его депрецировать, если не знать, чем заменить?
А причем тут то проектирование? Развешивают public/protected/private на методы и аттрибуты сами девелоперы, авторы кода. И у каждого своя идея как оно должно в идеале работать. И public/protected/private/deprecated более или менее единственное средство как они могут свой идеал до других донести.
На ютюбе как-то презентацию видел. Там CTO какой-то мелкой фирмы на тему почему у них в интерфейсах нету public/protected/private ответил "We are all adults here." Что в принципе и отражает мой сентимент. Политические игры с public/protected/private (и в жабе с deprecated) я видел только в ситуациах которые можно легко обобщить термином "детский сад".
private это же деталь реализации, она по определению не может быть частью публичного API :)
package private в яве и friend в крестах - тоже деталь реализации и очень годная вещь, позволяющая не выставлять лишнего в паблик.
Ну а protected... Тут спорная тема. Юзать его чтобы "разрешить вызывать метод только потомкам" или чтобы "дать потомку доступ к полям" - ССЗБ (привет, QThread::sleep()). А вот если юзать его как простенький service provider interface - почему нет? Поясню, что имел в виду:
- Сабклассы никогда не вызывают protected методы, а только перекрывают их
- Все остальные работают с публичными методами, и даже не думают об этих protected'ах
Это дает какую-никакую развязку между теми кто сабклассит и теми кто юзает. И это чуть проще, чем полноценный SPI.
Так что они абсолютно правы ;)
P.S. Я поди криво выразил свою мысль?
А вот правильный ответ на вопрос "зачем нужны приватные виртуальные функции в C++"
Т.е. protected нинужен?
P.S. Может кому-то пригодится: http://www.gotw.ca/publications/mill18.htm
И еще больше того что дублирует другой функционал.
Ну так это всё в рамках парадигмы: известно, что компилятор сначала выбирает функцию, а только потом проверяет уровни доступа. Поэтому, например, происходит вот такое:
Так и я про то же!!!
Все к чему public/protected/privatе/deprecated приводят это к лишним спорам.
Нужен гибкий способ подробного описания зоны видимости на запись и зоны видимости на чтение.
большая часть всех этих проблем происходит из-за того что народ копи-паздит откуда нипопади кривое использование класса/объекта, а потом менять не хочет потому что уже как бы протестили и зарелизили.
паблик - виноват тем что не дефолт.
В сишке то с пабликом и приватом все очень просто.
?
В ашке пишем только публичный интерфейс, и одно приватное поле с указателем на структуру, в которой хранятся поля.
Кутишники юзают эту хреновину для бинарной совместимости, ну и чтобы лишние ашки типа виндовых или линуксовых хидеров не торчали из их API.
Большей изоляции паблика и привата я не видел ;(
А, да, знаю. Так а другого способа в С++ как бы публичные интерфейсы делать и нету.
Сам то я коммерческий софт пишу. Тут совместимость нужна только на уровне компиляции, поэтому pimpl'ом ни разу в офисе пользоваться не приходилось.
Ну он все-таки для либ, не для прог.
pimp часто может неплохо увеличить скорость этой самой компиляции.
Да есть там способ... он даже идеологически правильней (с точки зрения ООП), но далеко не самый приятный, и не самый шустрый (хотя пимпл тоже не айс в плане удобства и скорости).
Интерфейсы с чистыми виртуальными функциями + фабричные методы, возвращающие эти интерфейсы в каком-нибудь смартпоинтере. Оп, оп, жаба стайл.
Можно.
Но в долгосрочных проектах это просто невозможно поддерживать, потому что добавлять в интерфейс новые (чисто) виртуальные функции нельзя. Приходится делать новый новый интерфейс (часто унаследованые от старого) и менять у пользователей имя интерфейса на новый. Вообщем, работает если только интерфейс стабильный.
Скажи это майкрософту с их com ;)
Само собой интерфейсы менять нельзя, только добавлять новые и выпиливать старые, когда все на них забьют (если забьют).
Насчет неудобств - внутри одной подсистемы неудобно, на границе между подсистемами - почему нет?
Бррр... Ты о чем?
На внутренних интерфейсах - это оверкил.
На внешних интерфейсах - морока поддерживать, и долго не выживает.
К слову у нас к паре либ С++-тых основной метод работы через интерфейс 80+ С-style функций. Пару лет по граблям двоичной совместимости походили, и решили откатится на проверенные методы.
Если не ком и не си стайл, то что? сокеты? общая память? сериализация?
В том что старые интерфейсы когда-нибудь надо удалять. В коммерческом софте это почти всегда значит "никогда".
"Qt вывели какой-то свод правил [...]"
Не Qt, а KDE:
http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++
в любом случае старый дропаешь и приложение больше не запускается.
потом через какое-то время старый интерфейс просто заменяете на заглушки с диалогом до свидания, ссылка на обновление.
А можно с этого места поподробнее? Я вот наоборот, плохо понимаю зачем прятать что-то от потомков.
Иначе потом имеем антипаттерн "паблик морозов".
А зачем прятать что-то от других, если оно доступно потомкам? :)
Вот этот QThread::sleep заныкали, когда идеология говорила "засабклассь тред и перекрой метод run()". А потом партия сказала "не надо сабклассить QThread, он не для того", а sleep() так и валялся в протектедах до пятой версии. И чтобы его юзать надо было есть кактус сабклассить тред чисто ради него.
А поля и прочие кишки давать потомкам - плохая идея. Потом будешь долго материться и рвать волосы, когда захочешь перепилить базовый класс.
Ну при архитектурных сдвигах по фазе уже ничто не поможет.
Имхо, имеют смысл три степени инкапсуляции.
1. Обычное инстанциирование - только паблик, чтобы не смущать пользователя кишками. Если класс не абстрактный, конечно.
2. Наследование с ожидаемым перекрытием методов - вот типа того-же Thread::run() во многих языках. Тоже паблик.
3. И наследование с серьёзными изменениями, не предусмотренными изначально. Тут protected, чтобы понапрасну обычные инстансы за эти кишки не дергали. Т.е. здесь держать только то, что менять можно, но только на свой страх и риск, и необходимость сабклассить должна служить тому предупреждением.
run() это не часть публичного API треда, это часть, которую тред требует от своих потомков. Нахрена тут паблик? Ты же никогда не будешь вызывать этот run() сам.
> наследование с серьёзными изменениями, не предусмотренными изначально
Бедные люди, которым потом поддерживать эти "серьезные изменения"...
Хм. Логично. Хотя мне нравится питоновская реализация, где по умолчанию Thread::run() просто вызывает заранее переданную необязательным параметром конструктора функцию.
Т.е. можно просто скормить тело потока реализации по умолчанию, а можно отсабклассить.
Впрочем, в этом случае паблик тоже незачем.
Убедил.
Спорная вещь, кстати. Явно показывающая, что наследник не является предком, а паразитирует на нем.
О чем думали: Блин, лень агрегировать хештейбл, столько методов ему пробрасывать придется... лучше я от него унаследуюсь и получу все нахаляву.
Что получилось (затыкаем дырки в абстракции путем их документирования): Because Properties inherits from Hashtable, the put and putAll methods can be applied to a Properties object. Their use is strongly discouraged as they allow the caller to insert entries whose keys or values are not Strings. The setProperty method should be used instead. If the store or save method is called on a "compromised" Properties object that contains a non-String key or value, the call will fail. Similarly, the call to the propertyNames or list method will fail if it is called on a "compromised" Properties object that contains a non-String key.
Типа такого (Питон т.к. Яву не знаю)
А оно расширяет Hashtable<Object,Object>, а до этого вообще тупо Hashtable. Поэтому эти put() и putAll() будут работать с Object'ами, и обрезать их до String'ов уже не получится. Разве что исключение в рантайме кидать. А компайл-тайм проверку проебали в любом случае.
Всё же скриптовые языки деформируют мышление.
В любом случае, наследование ради реализации, а не ради интерфейса, ещё никого до добра не доводило.
Пойми, ты не пользователя этими кишками пугаешь... Да пользователь будет рад-радёхонек ими воспользоваться при любом удобном случае, если это как-то облегчит ему работу.
А вот тебе потом будет плохо, ибо изменить эти кишки ты уже не сможешь, т.к. они стали частью паблик интерфейса и вросли в кучу говнокода, который всем вломы править.
Тут, конечно, можно отмазаться, мол, гарантируем обратную совместимость только на первых двух уровнях абстракции, если лезешь ниже - ССЗБ.
Но скорее всего такую либу пошлют и правильно сделают.
Ну депрекейтед как бы по определению должен предлагать альтернативу (UPD: чуть выше об этом уже написали)... Собственно "старый интерфейс как враппер поверх нового" это тоже своего рода deprecated и not recommended for new projects.
> 11 лет спустя, в проекте есть как минимум пять копий оного хэша
Ну дык можно было сделать этому генератору публичный API, когда он понадобился во втором месте... Причем, имхо, даже не выставлять этот генератор в паблик у того модуля, а вынести в отдельный, дабы не создавать ложную зависимость. Тот кто его изначально сделал приватным, имхо, не виноват, и делал все правильно.
> Старый интерфейс просто делается как враппер поверху нового.
А тут согласен.
Сделать можно все. Но разраб сказал что хэш функцию никому больше не надо знать - и точка.
Назначение видимости public вполне соответвует паттерну High Cohesion, когда мы группируем обязанности и распределяем их между несколькими специализированными обработчиками, так что нельзя сказать, что это само по себе говно.
это как если бы ваша жена пошла бы на панель.