Pull to refresh

Comments 67

Если пишешь код C++ то в голове даже мысли быть не должно о возможности использования кастов в стиле С, только касты С++.
P.S. Может тему лучше в топик C++ перенести?
Вообще надо уже просто ставить в компиляторе варнинг, который будет кидаться при виде c-style cast
Большинство разработчиков все равно его выключит, чтобы «не засорялась» выдача компилятора.
Поставить -WError или /Wx и отправить на исправление ;)
Причем обязательно на билд-сервере, а разработчик — пускай уж ставит как хочет:)
Поэтому на него и не кидается варнинг по умолчанию ;)
Вообще надо уже отбирать интернет-паспорт за «C/C++».
%Комментарий о том, что в C# такие проблемы отсутствуют как класс%

%Комментарий о том, что надо бы почаще использовать ATL, помогающий в целом ряде вопросов с COM-интерфейсами%
>почаще использовать ATL
Совершенно справедливо, ATL — отличная штука, но это не отменяет необходимости думать и правильно пользоваться средствами языка.

Например, может возникнуть потребность выводить в журнал все запросы на интерфейс. Для этого в ATL есть макрос COM_INTERFACE_ENTRY_FUNC, который позволяет перенаправить запрос в функцию с предопределенной сигнатурой, которая может делать что угодно. В этой функции придется заново реализовать значительную часть того, что делает реализация выше, и опять есть риск посадить труднообнаружимую ошибку, если неправильно использовать приведения.
Спасибо за макрос, не знал.
Можно было бы и покороче пример привести ради такой морали ,)
P.S. Если не секрет, что за изысканный комментарий (// V2UncmUgaGlyaW5nIC0gd3d3LmFiYnl5LnJ1L3ZhY2FuY3k=) во втором QueryInterface?
В общем-то здесь явный косяк в проектировании. Ну не должен класс от 10 интерфейсов наследоваться. Я вообще если вижу, что класс более, чем от двух объектов унаследован, то начинаю думать.
В целом, справедливо, но точно так же можно ошибиться и при 5 интерфейсах, а 5 интерфейсов для COM-объекта — вполне обычное дело. Скажем, IUnknown+IDispatch+ISupportErrorInfo плюс пара каких-нибудь специфичных для этого объекта — уже 5. А специфичных может быть и больше. Просто не стоит забывать, что есть интерфейсы, которые по тем или иным причинам вынужден реализовывать каждый объект.
Поэтому я и обхожу штуки типа COM'а подальше, благо возможность у меня есть.
А ну-ка расскажите штуку, дающую возможность создать под Windows компонент, который можно заюзать из любого языка программирования, из под виртуальных машин JAVA и .NET, из Powershell-скриптов, удаленных машин, из макросов офиса и скриптов AutoIt и кучи других мест.

Ответ «а я не пишу под Windows» — правильный, но не спортивный.
dbus, таки он кроссплатформенен и я проверял, вполне сносно работает в винде
Да ну?
Представьте, что вы делаете игру про эльфов с орками.
Скажем, объект «орк» должен наследоваться от следующих классов:
1. графический объект, умеющий себя рисовать
2. игровой объект, хранящий «статы» типа силы, ловкости и т.п. и участвующий в обсчете механики
3. сохраняемый объект (для сейвов)
4. Объект, управляемый AI
5. -и/или- объект, управляемый игроком
6. Объект, служащий целью для ударов, заклинаний, разговоров и т.п.
7. Объект, участвующий в квестах (с ним надо поговорить, или убить, или что-то дать, ...)

Все эти 7 классов пересекаются, но ни один из них не вложен в другой класс. И? «Ну не должен наследоваться»?
man стратегии. У Александреску кстати очень похожий пример был разобран, правда там на примере игры doom было. Как раз наследование в этом конкретном случае — самое зло.
Ололо, я хочу посмотреть на реализацию упомянутого выше орка на чистых стратегиях, без наследования. И рядышком на реализацию эльфа. И еще на реализацию квестовой бутылки рома. Хотя бы просто заголовки классов. Сколько у вас будет параметров в шаблонах? 50? 100?
Книги Александреску, Майерса и т.п. — не детективы, их нельзя читать «по диагонали». Тут надо думать над каждым прочитанным словом. В вашем «современном проектировании» есть маленький абзац про то, что шаблоны (стратегии) и виртуальные функции (множественное наследование) не заменяют друг друга, а дополняют. Да-да, есть большая пересекающая область задач, которые можно решить обоими механизмами, но есть и уникальные вещи. Вы не можете выполнить статическое связывание (стратегии) в ситуации, когда вы не знаете, с каким конкретно объектом будете работать. В данной статье — как раз та самая ситуация.
PS. Пример Александреску про doom как-то не видел, поделитесь линком если можно
так совсем то наследование не нужно убирать, но нужно избегать его и не городить супер интерфейсов
Скажем, объект «орк» должен наследоваться от следующих классов:

Зачем? Более разумных способов нету, чем пихать всё и вся в один объект?
Понимаете, наш «орк» — это сложная сущность, и ничего вы с этим не сделаете. Он должен «уметь» отрисовываться, управляться, участвовать в боях и далее по списку. Вы, конечно, можете искусственно «распилить» большой класс на много мелких — с помощью указателей или вложенных объектов, вы также можете «размазать» большие методы по куче вспомогательных классов и шаблонов, если вы боитесь длинных файлов. Но проблема в том, что у любой декомпозиции есть предел, когда дальнейшее уменьшение размеров классов или процедур приводит к усложнению кода, а не к его упрощению. И этот орк — как раз тот случай, когда любое дальнейшее его упрощение — кроме наследования, будет приводит к существенному усложнению кода. Потому что отдельные «интерфейсы» уже не независимы, они должны знать друг о друге. Отрисовка модели меняется по мере прохождения квестов, действия AI сильно зависит от статов и так далее. Если вы вынесете статы в один объект, AI в другой, а 3D модель в третий, вы никаких бонусов кроме дополнительного геморроя не получите.
Я не знаю почему, но те, кто пишут книги по ООП, очень сильно бояться больших классов и методов. Ну, когда работаешь только с простыми, в общем-то, вещами типа стандартных контейнеров или элементов UI, это оправдано. Заставить бы их написать и отладить модель какого-нибудь Навье-Стокса методом Маккормака в криволинейных координатах — глядишь может глаза бы и открылись.
Все имхо разумеется, просто надоело вправлять ООПухоли мозга
есть другая сторона медали.
когда в результате такого ООП-буйства нам чтобы всего-навсего взять содержимое элемента дерева нужно городить что-то вроде:

dynamic_cast<const xmlpp::Element*>
(*((Parent->get_children(Item)).begin()))->get_child_text()->get_content()


это пример для чудесной библиотеки libxml++ с которой мне пришлось столкнуться.
чтобы составить эту чудесную формулу и загнать ее в макроопределение у меня ушло полдня)
Вы говорите «другая сторона медали», но не совсем понятно в чём именно у вас противопоставление с предыдущим оратором?
Не, ну «те, кто пишут книги по ООП» наверное не просто так «боятся», и занимались не только контейнерами и UI :) Например, говоря только про упрощение кода вы упускаете ещё один важный фактор — переиспользование. Мега-класс будет гораздо сложнее переиспользовать, чем правильно декомпозированный на более простые сущности, даже если они сильно связаны. Конечно, если переиспользовать всё равно не получится по некоторым причинам — то это уже другой вопрос.
Просто надо разделять типы кода. Библиотеки отдельно, с максимальной гибкостью, универсальностью и расширяемостью, по всем канонам ООП и повторным использованием. С конечными классами предметной области все наоборот — надо решать конкретную задачу и забыть про «повторное использование». Вы никогда не будете повторно использовать вашего орка в другом проекте, прямо вот чтобы взять целиком готовый класс и куда-то его воткнуть без изменений. В любом случае будет глубокий рефакторинг. Сидеть и гадать, какие части возможно изменятся в будущем, а какие удастся перенести в будущий проект без изменений — это извините bullshit
В целом-то я согласен. Но во-первых, это зависит от конечных классов. Иногда сразу очевидно, что «эту вещь мы сможем использовать и в других местах, как бы её сделать более независимой...». Может с вашими «орками» и не так, нужно смотреть в каждом конкретном случае. А во вторых, это ещё и вопрос опыта — неопытному разработчику конечно не имеет смысла «сидеть и гадать», а опытный может сразу видеть, какие вещи можно переиспользовать, а какие — нет. Естественно, речь не о том, чтобы ваш мега-класс целиком взять, а вот какие-то компоненты — почему бы и нет, если они не перемешаны с остальным кодом в ацкое месиво.
Единое название куче сущностей «орк» вы дали сами. Интуитивность при разработке крайне редко приводит к хорошим решениям. Именно поэтому с ООП столько проблем (все хотят объект «мошына»), а не с самим подходом.
> Все имхо разумеется, просто надоело вправлять ООПухоли мозга
Мне лично нравится ФП. То, где нет наследования и сопутствующих проблем.
Просто не присваивайте в void*, а все остальное за вас сделает компилятор безо всяких кастов :)
При использовании COM сплошь да рядом наследование от 3 и выше интерфейсов. Мне не кажется, что это так уж плохо
>но такие же проблемы могут возникать в любых достаточно сложных иерархиях классов, если не уделять достаточно внимания декомпозиции

fix
Когда вы пишете все приложение и можете сами контролировать иерархию классов, — да. Но иногда вы вынуждены работать с уже существующей иерархией и вынуждены реализовывать интерфейсы, предопределенные третьей стороной.

Неспроста же там написано «могут возникать», а не «будут возникать».
Ну тут конечно, единственное возможное решение — это врубить варнинг, а еще лучше ошибку, при виде c style cast'а и везде их заменить на static_cast'ы, ну или reinterpret_cast'ы, если точно есть уверенность в том, что так надо.
А dynamic_cast вообще же из другой оперы.
Но в целом да, не зря в умных книжках советуют по возможности избегать наследования.
а зачем прописывать всё вручную? варианты со стандарными решения защищающими от копипасты по каким-то причинам не проходят?
BEGIN_COM_MAP(CYetOtherImplementor)
	//COM_INTERFACE_ENTRY(IComInterface2) //активируйте и получите ошибку
	COM_INTERFACE_ENTRY(IComInterface3)
	COM_INTERFACE_ENTRY(IComInterface4)
....
	COM_INTERFACE_ENTRY(IComInterface10)
END_COM_MAP()

Если попытаетесь использовать интерфейс которого нет в базовых у CYetOtherImplementor будет вам предупреждение
Да, и этого достаточно, пока вам не понадобится записывать в журнал каждый запрос, а тогда вы, скорее всего, используете COM_INTERFACE_ENTRY_FUNC для перенаправления в свою функцию извлечения интерфейса. Кстати, об этом комментарий выше.
Реализуйте логику логирования в своем хуке, но не переделывайте логику получения интерфейса. Вызовите далее по цепочке стандартный InternalQueryInterface, как это делает COM_INTERFACE_ENTRY_CHAIN
Какой класс вы предлагаете указать в качестве параметра COM_INTERFACE_ENTRY_CHAIN?
Никакой, я говорю про принцип.

Для того чтобы отследить все запросы определенного интерфейса делаем так:

было:
BEGIN_COM_MAP(CYetOtherImplementor)
...
     COM_INTERFACE_ENTRY(IComInterface3)
...
END_COM_MAP()

стало:
BEGIN_COM_MAP(CYetOtherImplementor)
...
     COM_INTERFACE_ENTRY_FUNC(IID_IComInterface3,offsetofclass(IComInterface3, _ComMapClass),myfunc)
...
END_COM_MAP()

static HRESULT WINAPI myfunc(void* pv, REFIID riid, LPVOID* ppv, DWORD_PTR dw)
{
	//do some loggin for params

	//default behavior
	IUnknown* pUnk = (IUnknown*)((INT_PTR)pv+dw);
	pUnk->AddRef();
	*ppv = pUnk;
	return S_OK;
}
Где же здесь что-либо похожее не вызов InternalQueryInterface()?

В целом, предлагаемое вами решение приемлемо, но имеет существенный недостаток — в нем очень много приведений и манипуляций с указателями, в которых легко запутаться и с которыми компилятор не поможет. Например, отступы от начала объекта, если окажутся неправильными, нужно очень долго анализировать. Видите вы в журнале выдачу, где отступ равен 18, — и что, это признак проблемы или нет?

Можно вместо этого сделать шаблонную функцию, параметризованную запрашиваемым интерфейсом, в ней выводить имя конкретной функции (макрос __FUNCTION__ поможет), проверять, что параметр «идентификатор интерфейса» имеет правильное значение, приводить указатель pv (кстати, в ATL он называется хотя бы pThis) сначала к классу объекта (static_cast), потом — к интерфейсу (еще один static_cast). Снова нужно реализовать значительную часть QueryInterface() и снова так, чтобы минимизировать риск ошибки и максимально упростить отладку.
riid содержит тот интерфейс который хотят запросить, проверок на «идентификатор интерфейса» никаких не надо, всё проверено до этого и именно потому попало в нужную вам функцию.

>>Где же здесь что-либо похожее не вызов InternalQueryInterface()?
файл atlcom.h хоть раз в жизни открывали? до atlbase.inl проходили?
То что вы описываете называется велосипедостроением и не знанием основ работы COM.

хотите ослеживать всё — вставьте в начало карты блайнд функцию, сохраняйте всё что угодно.

BEGIN_COM_MAP(CYetOtherImplementor)
	//следим за всеми интерфейсами, даже теми которые мы не поддерживаем
	COM_INTERFACE_ENTRY_FUNC_BLIND(0,myfunc)  //самый первый элемент в карте
...
	//размещаем выше нужного интерфейса если нужно следить только за ним
	//COM_INTERFACE_ENTRY_FUNC(__uuidof(IComInterface3),0,myfunc_3) 
...
	COM_INTERFACE_ENTRY(IComInterface1)
	COM_INTERFACE_ENTRY(IComInterface3)
...
	COM_INTERFACE_ENTRY(IComInterface10)
...
END_COM_MAP()

static HRESULT WINAPI myfunc(void* pv, REFIID riid, LPVOID* ppv, DWORD_PTR dw)
{
	//do some loggin for params
	//riid -> clsid -> bstr, progid
	return E_NOINTERFACE; //заставить работать далее по карте
}

static HRESULT WINAPI myfunc_3(void* pv, REFIID riid, LPVOID* ppv, DWORD_PTR dw)
{
	//do some loggin for params	
	//riid always == __uuidof(IComInterface3)
	return E_NOINTERFACE; //заставить работать далее по карте
}
*) в myfunc_3 вернуть надо не E_NOINTERFACE, а S_FALSE для продолжения обработки
Да, можно, но все равно не узнаете, что произошло после этого. Как вариант, если уровень паранойи не очень высокий, можно делать и так.
После этого был запрос интерфейса. потому что мы в фунции запроса интерфейса.

Хотите узнать удачным ли он был? блайнд функция в конец, попали в неё, запрашиваемый интерфес не нашли.
Когда ваш объект используется из процесса, о работе которого вы вообще ничего не знаете, чем больше в вашем коде паранойи, тем быстрее отладка. Чем принципиально хуже шаблонная функция, которая проверяет все параметры и либо сама извлекает интерфейс и пишет в журнал, что она его извлекла, либо пишет ясное сообщение об ошибке и возвращает код, обозначающий ошибку?
в вашем объекте — ваш код, а ваш код включает ваши заголовки, а в ваших заголовках вся ваша параноя и проверки все внутри вашего кода уже есть из шаблонов.
Каждый сам решает, сколько паранойи ему нужно в его коде, не правда ли? Решение использовать шаблонную функцию было принято сознательно — в первую очередь для того, чтобы иметь предельно подробную выдачу в журнал. Можно ли его считать изобретением велосипеда просто потому, что заново написан код извлечения интерфейса? Очень может быть.
ваша параноя вносит ошибки которые вы описали в статье. правильное использование стандартных средств таких ошибок не дает и не требует множественного перечисления интерфесов в разных местах.
«предельно подробная выдача» — это лукавство. Вы в своем методе имеете не больше информации чем внутри ENTRY_FUNC или блайнда.
>файл atlcom.h хоть раз в жизни открывали? до atlbase.inl проходили? Открывали, и не только этот файл, но и тот, из которого вы скопировали <a href=«habrahabr.ru/company/abbyy/blog/113429/#comment_3644177>»default behavior"

>хотите ослеживать всё — вставьте в начало карты блайнд функцию, сохраняйте всё что угодно.
Да, но как узнать, что произошло потом? Чтобы знать точно, нужно выводить также и вызовы конкретных функций запроса. Поэтому blind-функцию надежнее ставить только в самом конце «карты», когда уже точно не было возможности извлечь интерфейс.

В то же время, если вернуть E_NOINTERFACE (или любой код, для которого FAILED дает «истину») из обычной (не blind) функции запроса, код, вызвавший эту функцию запроса, перестанет перебирать карту — расчет на то, что раз фунцию вызвали, она должна извлечь интерфейс успешно, в противном случае что-то пошло не так и дальше пытаться бессмысленно.

>riid содержит тот интерфейс который хотят запросить, проверок на «идентификатор интерфейса» никаких не надо, всё проверено до этого и именно потому попало в нужную вам функцию.
Он должен содержать тот интерфейс, и это стоит проверить, тем более что это совсем нетрудно.
riid будет содержать его, потому что функция не блайнд и он уже проверен, иначе он не попал бы в указанную функцию
if (bBlind || InlineIsEqualGUID(*(pEntries->piid), iid))
{
	if (pEntries->pFunc == _ATL_SIMPLEMAPENTRY) //offset
	{
		ATLASSERT(!bBlind);
		IUnknown* pUnk = (IUnknown*)((INT_PTR)pThis+pEntries->dw);
		pUnk->AddRef();
		*ppvObject = pUnk;
		return S_OK;
	}
	else //actual function call
	{
		HRESULT hRes = pEntries->pFunc(pThis,
			iid, ppvObject, pEntries->dw);
		if (hRes == S_OK || (!bBlind && FAILED(hRes)))
			return hRes;
	}
}
Да, если в «карте» действительно был верный идентификатор. При задании карты тоже можно ошибиться.
Читаем первый комментарий, при ошибке в карте будет ошибка компиляции.
Если вы используете COM_INTERFACE_ENTRY_FUNC, то никто не проверит, что вы туда передали, соответствие идентификатора и функции будет на вашей совести.
Вы или не понимаете что пишете или не знаете какую проблему решаете.
COM_INTERFACE_ENTRY_FUNC(my_left_iid,0,myfunc)

приведет к тому что myfunc вызовется только когда riid интерфейса который пытаются запросить будет соответствовать my_left_iid и проверка его внутри функции всегда будет срабатывать, потому что с другим iid ваша функция не вызовется.

если же вместо второго параметра указывать
offsetofclass(IComInterface3, _ComMapClass)

то будет и проверка компилятора

считаю продолжение диалога бесмысленным.

Переаттестация — отличный способ определить реальный уровень технической грамотности вашего отдела разработки ПО.
Что мне не нравится в static_cast, так это то, что оно смешивает безопасное приведение, например, к базовому типу, с небезопасным — от базового класса к потомкам. В вашем случае, между прочим, вы как раз просто приводите к базовому типу, так что можно обойтись вообще без кастов:
IComInterface1* p = this;
*ppv = p;


А ещё, само по себе появление IComInterface1 дважды, один раз в __uuidof и второй раз при приведении — это уже плохо, уж слишком просто облажаться при копипасте. На мой взгляд, это один из случаев, когда макросы не зло, а совсем даже наоборот.
>смешивает безопасное приведение, например, к базовому типу, с небезопасным — от базового класса к потомкам
Справедливо, в описанном случае безопасность static_cast основана как раз на том, что this — точно указатель на объект производного класса, у которого нет наследников.

> вы как раз просто приводите к базовому типу, так что можно обойтись вообще без кастов:
>IComInterface1* p = this;
>*ppv = p;
Да, мысль очень правильная. Практически убедить разработчиков приводить так будет проблематично — очень параноидально выглядит эта конструкция.

>само по себе появление IComInterface1 дважды, один раз в __uuidof и второй раз при приведении — это уже плохо, уж слишком просто облажаться при копипасте.
В целом — да, но в данном случае это соседние строки, все же после копипаста надо хотя бы раз прочитать, что получилось, иначе постоянно будут возникать очень глупые ошибки. В исходном примере ошибка появилась во многом из-за того, что объявление класса и соответственно список базовых классов находится очень далеко от реализации QueryInterface() и ошибиться намного легче.
>Да, мысль очень правильная. Практически убедить разработчиков приводить так будет проблематично — очень параноидально выглядит эта конструкция.

Нормальная конструкция, когда занимался перегрузкой операторов, часто её использовал
Можно завернуть в шаблонную функцию, тогда не будет этой «лишней» промежуточной переменной и удобнее использовать в коде.
Э. Трельсен — Модель COM и применение ATL 3.0", стр. 139:

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

Теперь, если вызывающая сторона передаст адрес указателя, в котором хранится ненулевое значение и, например, __uuidof(IDraw), в *ppv останется то ненулевое значение и вызов AddRef() приведет в неопределенному поведению.
Не об этом речь.
То обстоятельство, что лучше использовать static_cast, давно описано в «правильных» книжках.
Да, «правильная книжка» приводит пример с неверной реализацией QueryInterface(). Отлично.

Вот пример реализации IUnknown::Release() из другой «правильной книжки»:

ULONG __stdcall CA::Release()
{
    if (::InterlockedDecrement(&m_cRef) == 0)
    {
        delete this;
        return 0;
    }
    return m_cRef;
}


видите, даже InterlockedDecrement() используется. Но здесь возможна ситуация, когда метод будет вызван двумя потоками одновременно, первый поток получит от InterlockedDecrement() единицу, второй — ноль, и второй поток выполнит delete раньше, чем первый прочитает поле класса, возникнет неопределенное поведение.

Из того, что что-то написано в «правильной книжке», не следует, что это правильно и многим очевидно. Нужно не только внимательно читать книги; думать и подвергать свои знания сомнению тоже не вредно.
Sign up to leave a comment.