|
Программирование >> Обобщенные обратные вызовы
Имеются также две механические проблемы. Первая заключается в том, что член currtype открыт без существенных на то причин; это нарушает принцип инкапсуляции и означает, что любой пользователь может внести путаницу во флаги, пусть даже ненамеренно. Вторая механическая проблема связана с именами, использованными в перечислении; об этом я скажу позже, в отдельном подразделе. protected: Здесь мы сталкиваемся со второй механической ошибкой: внутреннее устройство класса должно быть закрытым, а не защищенным. Единственная причина, по которой оно может быть зашишснным, - это необходимость обеспечить доступ со стороны производных классов. Что касается данного случая, то лучше не иметь никаких производных классов, так как наследовать myunion небезопасно по целому ряду причин -- хотя бы из-за странных игр с внутренним устройством класса и отсутствия виртуального деструктора. > Рекомендация Всегда делайте все члены-данные закрытыми. Единственным исключением является случай структур в стиле С, которые не предназначены для инкапсулирования и все члены которых являются открытыми. union { int i; unsigned char buff[max(sizeof(LIST),sizeof(STRING))]; } U; void cleanupO; Ha этом оканчивается определение главного класса. Пойдем дальше и рассмотрим три параллельные функции доступа. inline int& MYUNION : :getintO ifС currtype== lNT ) { return u.i; } else { cleanup О; currtype= iNT; return u.i; } else inline LI5T& MYUNION::get!iSt О if( currtype== LiST ) { return *(reinterpret cast<Ll5T*>(U.buff)); } else { cleanupO; list* ptype = new(u.buff) listO; currtype= LiST; return *ptype; } else inline STRINGS MYUNION::getstring О ifС currtype== STRiNG) { return *(reinterpret cast<STRiNG*>(u.buff)); } else { cleanup О; STRING* ptype = new(U.buff) STRING О; currtype= STRING; return *ptype; } else Маленькое замечание: комментарий el se ничего не дает и совершенно бесполезен. > Рекомендация Пишите (только) полезные комментарии. Никогда не пишите комментарии, которые повторяют код. Комментарии должны пояснять код и причины, по которым вы написали его именно так. Но здесь еще есть три более серьезные проблемы. Первая заключается в том, что функции не написаны симметрично, и в то время как первое использование списка или строки дает объект, построенный при помоши конструктора по умолчанию, первое использование int дает нам неинициализированный объект. Если это сделано преднамеренно, чтобы отразить обычную семантику неинициализируемых переменных типа i nt, то это следовало документировать; в противном случае int также должен быть инициализирован. Например, если вызывающая функция обращается к geti nt и пытается сделать копию (неинициализированного) значения, го в результате мы получаем неопределенное поведение - не все платформы поддерживают копирование произвольных некорректных значений int и могут отвергнуть такие инструкции в процессе работы программы. Вторая серьезная проблема состоит в том, что данный код не является корректным с точки зрения использования const. Корректный исходный текст, как минимум, содержал бы const-перефузки для каждой из рассматриваемых функций; все они, естественно, возвращают то же значение, что и их неконстантные аналоги, но как ссылки на const. > Рекомендация Последовательно и корректно используйте модификатор const. Третья проблема заключается в хрупкости такого подхода при необходимости внесения изменений. Он основан на переключении типов, и поэтому очень легко случайно рассинхроиизировать функции при удалении или добавлении нового типа. Теперь остановитесь и задумайтесь: как бы вы поступили при необходимости добавить новый тип? Перечислите по возможности полно все необходимые для этого действия. Вы готовы? Давайте сравним наши результаты. Чтобы добавить новый тип, вы должны помнить о том, что: надо добавить новое значение в перечисление; надо добавить новую функцию доступа; надо обновить функцию cleanup для удаления нового типа; и надо добавить этот тип в вычисление размера буфера, чтобы он был достаточно большим для хранения всех типов, включая новый. Если вы ошибетесь и пропустите что-то - что ж, .это будет отличной иллюстрацией того, насколько сложно поддерживать и расширять такой исходный текст. Перейдем к заключительной части. void MYUNION::cleanup О switch( currtype ) { case list: { list& ptype = getlistO; ptype.-LIST О; break; } case case STRING: { STRINGS ptype = getstringO; ptype.-STRING О; break; } case default: break; } switch currtype=NONE; Здесь опять можно высказать уже знакомое замечание по поводу комментариев - case и swi tch не несут смысловой иафузки. Лучше не иметь комментариев вовсе, чем такие комментарии, которые только отвлекают внимание. Но здесь есть и более серьезный вопрос: вместо простого default: break; было бы лучше использовать полный список типов (включая int) и сигнализировать о логической ошибке в случае неизвестного типа - вероятно, посредством assert или throw std::logic error(...);. И вообще, переключение типов является злом само по себе. Поиск switch С++ Dewhurst в Google дает массу ссылок на эту тему, включая [Dewhurst02. Если вас интересует более детальная информация по этому вопросу, чтобы убедить коллег не использовать эту методику, - обратитесь к найденным ссылкам. > Рекомендация Избегайте переключения типов; предпочитайте безопасность с точки зрения использования типов. Эти хитрые имена Есть еще одна механическая проблема, которую я еще не раскрыл в полном объеме. Впервые она проявляет себя во всей красе (вернее, уродстве) в строке enum uniontype {NONE, INT, LIST, STRING}; Никогда, никогда, вы слышите? - никогда! - не используйте имена, которые начинаются с подчеркивания или содержат двойное подчеркивание, эти имена зарезервированы для исключительного использования вашим компилятором и разработчиками стандартной библиотеки, чтобы избежать столкновения с такими же именами в вашем коде. Не трогайте их имен, и они не будут трогать ваши! * Не останавливайтесь! Читайте дальше! Вы могли встретить этот совет раньше. Вы могли даже услышать его от меня или прочесть в моих книгах. Это могло настолько вам надоесть, что, зевнув, вы решили перейти к следующему подразделу. Так вот, это не просто теоретические измышления! Строка с определением enum компилируется большинством компиляторов, которыми я компилировал данную программу: Borland 5.5, Comeau 4.3.0.1, gcc 2.95.3 / 3.1.1 / 3.4. Intel 7.0 и Microsoft Visual С++ от 6.0 по 8.0 (2005) beta. Но два из них -Metrowerks CodeWarrior 8.2 и EDG 3.0.1 со стандартной библиотекой Dinkumware 4.0 - не смогли с ней справиться. Если быть более точным, то правило гласит, что любое имя с двойным подчеркиванием в любом месте, такое как like thi s, или начинающееся с подчеркивания и прописной буквы наподобие Li keThi s. является зарезервированным. Если хотите - запомните это правило, но, на мой взгляд, проще просто полностью избегать двойных подчеркиваний и имен, начинающихся с подчеркивания.
|
© 2006 - 2024 pmbk.ru. Генерация страницы: 0
При копировании материалов приветствуются ссылки. |