Здравствуйте, SV., Вы писали:

SV.>Было херово — стало херово. Что такого особенного в воронах, чтобы их считать отдельно? Нужно воспользоваться подходящим классом коллекции, который поддерживает агрегирующие функции. Из того фреймворка, которым вы пользуетесь. Для дотнета это List<> и LINQ, для плюсов — например, STL'евские контейнеры и for_each.

в заголовке темы тег [С++] значит никаких LINQ и extension-методов

SV.>задачи типа подсчета перьев воронов одного типа решаются так:

SV.>

SV.>var ravens = ...;
SV.>var totalFeathers = ravens.Where(x => x.RavenType == RavenType.White).Sum(x => x.FeatherCount);
SV.>

это замена алгоритма O(1) когда перья уже сосчитаны на O(N) когда их надо считать каждый раз по-новой.

В C++ нет extension-методов, и единственный способ поменять поведение контейнера — это агрегировать его в свой класс

В С++ нет IEnumerable, по этому мы не можем написать

IEnumerable ravens = ...;

мы должны указать конкретный контейнер, например vector<Raven> что сразу вытаскивает деталь реализации в интерфейс, мы уже не можем свободно поменять контейнер например на set или map, не можем поменять значение хранимое в контейнере (без динамического полиморфизма)
(есть boost::any_range, но не уверен что там будет просто поменять set на map)

Здравствуйте, Abyx, Вы писали:


A>я смотрю на это и вижу кучу строк кода от которых можно избавиться.
A>я хочу заменить агрегацию наследованием чтоб выкинуть "ненужные" строчки кода

Угу. Я тоже вижу кучу лишних строчек кода. Только вот агрегацию на наследование менять не хочу. Меня больше смущает наличие setCount, делегируемый счетчику, и update/remove, делегируемые RegistryImpl. Отсюда и вопрос — счетчики должны меняться при изменении списка ворон? И почему можно внешним вмешательством перевести счетчики в невалидное состояние? Так что changeCount и setCount из этого интерфейса стоило бы отломать.

Далее, wasUpdated относится к счетчикам или ко всему реестру? При добавлении лысого ворона она правильно изменит свое значение? Т.е. обновится или нет? Ну и reset в clear здесь бы переименовать. Вообще, список воронов и статистика по ним — это разные объекты. Их то как раз можно выделять с явным интерфейсом:

struct RavensWithStatistics {
  Registry ravens;
  PlumesCounter stats;
}

Здесь Registry и PlumesCounter — абстрактные классы (фактически "интерфейсы") предоставляемые библиотекой. Логикой создания может заниматься отдельная функция (или несколько функций с разными типами параметров и т.п.). Реализация "интерфейсов" (конкретные наследники) могут вполне скрывать какие-угодно детали реализации.

Еще вопрос. А зачем писать "отдельный фасад", если все методы можно перенести в RegistryImpl и не мучиться? Он же все равно о счетчиках знает, пусть их и делегирует. Да и без счетчиков он бесполезен.

Далее, зачем привязка классов к предметной области? Вот почему классы называются по тому, как они будут использоваться в коде, а не по тому, что они делают? Вот PlumesCounter. Это же не PlumesCounter. Это вообще набор абстрактных счетчиков. Вот и стоит его называть CountersGroup (или как-то похоже) и класть в общую библиотеку. А то будут счетчики перьев для птиц, счетчики чешкек для рыб и т.п. С RegistryImpl еще непонятнее. Он точно должен быть отдельным классом и инкапсулировать какую-то логику внутри? Или вам просто нужно было к колекции добавить пару методов (tryGetType и findAnyOfType)?. Кстати, зачем вообще RegistryImpl получать количество перьев, ведь его получить никак нельзя . Так что при желании вместо этого класса берется стандартная коллекция, пишется несколько методов, работающих с коллекцией птиц (поиски ваши) и к этой конструкции пишется фасад.

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

Здравствуйте, Abyx, Вы писали:

A>есть какая-то задача, например слежение за воронами
A>надо
A> 1) вести учет ворон,
A> 2) считать суммарное число перьев у ворон одного типа

A>для этой задачи написан класс который делегирует работу двум другим классам, решающем соответствующие подзадачи

[skip]

Для начала, у тебя тут путаница из-за употребления терминов. Impl-классы как правило агрегируют по ссылке или по указателю, иначе нет смысла давать такое абстрактное название. То есть идиоматическая реализация выглядит примерно так:

// CorvusRegistry.h

class RegistryImpl;

class CorvusRegistry
{
public:
  CorvusRegistry();
protected:
  RegistryImpl *pRegistry_;
};

Где-то за два каталога от CorvusRegistry.h:

// CorvusRegistry.cpp
#include "RegistryImpl.h" // << не доступен из CorvusRegistry.h

CorvusRegistry::CorvusRegistry() : pRegistry(new RegistryImpl){}

Иными словами, Impl-класс нужен для того, чтобы скрыть детали реализации от пользователей интерфейсного класса (в данном случае — CorvusRegistry). А у тебя получается так, что RegistryImpl виден всем.

Либо RegistryImpl должен иметь какое-то более конкретное название, скажем, FileRegistry, тогда его можно без лишнего удивления агрегировать по значению любым методом: хоть агрегацией, хоть наследованием.

A>я смотрю на это и вижу кучу строк кода от которых можно избавиться.
A>я хочу заменить агрегацию наследованием чтоб выкинуть "ненужные" строчки кода
A>

A>struct CorvusRegistry : PlumesCounter, RegistryImpl
A>{
A>    CorvusRegistry() { regImpl.setCounter(*this); }
A>};
A>

Если применять наследование именно по такому сценарию — для "упрощения" делегирования, то возможна одна неприятность: если изменится контракт любого базового класса, это изменение может отразиться на пользователях CorvusRegistry. Это может быть багой, а может быть фичей, зависит от способа использования CorvusRegistry.

A>прав я или не прав?

На мой взгляд, в данном случае так делать не надо, появятся лишние зависимости пользовательского кода от RegistryImpl и PlumesCounter. Но "не надо" не означает "нельзя": вполне возможно, что в твоём случае эти самые самые лишние зависимости не страшны.

Здравствуйте, maxkar, Вы писали:

M>Угу. Я тоже вижу кучу лишних строчек кода. Только вот агрегацию на наследование менять не хочу. Меня больше смущает наличие setCount, делегируемый счетчику, и update/remove, делегируемые RegistryImpl. Отсюда и вопрос — счетчики должны меняться при изменении списка ворон? И почему можно внешним вмешательством перевести счетчики в невалидное состояние? Так что changeCount и setCount из этого интерфейса стоило бы отломать.

гхм.. действительно, посмотрел — у меня в проекте Counter::changeCount используется только внутри RegistryImpl, а Counter::setCount не используется вообще %)
видимо я добавил их на всякий случай, а случай не случился =\

M>Далее, wasUpdated относится к счетчикам или ко всему реестру? При добавлении лысого ворона она правильно изменит свое значение? Т.е. обновится или нет?

только к счетчикам, вообще это сомнительный метод, он нужен для оптимизации обновления GUI, чтобы "когда летит стая ворон", не уведомлять GUI о каждой, а в конце проверить были изменения или нет, при чтении оно сбрасывается

M>Вообще, список воронов и статистика по ним — это разные объекты. Их то как раз можно выделять с явным интерфейсом:
M>

M>struct RavensWithStatistics {
M>  Registry ravens;
M>  PlumesCounter stats;
M>}
M>

M>Здесь Registry и PlumesCounter — абстрактные классы (фактически "интерфейсы") предоставляемые библиотекой. Логикой создания может заниматься отдельная функция (или несколько функций с разными типами параметров и т.п.). Реализация "интерфейсов" (конкретные наследники) могут вполне скрывать какие-угодно детали реализации.
Таки нет. веткой выше предлагали вообще не делать Counter, а если надо посчитать число перьев — вызывать std::count_if
И именно так я и хотел в начале сделать. Был бы 1 класс Registry и он бы считал количество перьев когда его попросят.

Но я изза любви к преждевременным оптимизациям подумал что надо бы что-ли мемоизацию приделать, а то и отдельный класс со счетчиками. И выделил из реализации Registry отдельный класс со счетчиками.
Но для остального кода ничего не поменялось, да ему и без разницы как Registry получает свои значения.

M>Еще вопрос. А зачем писать "отдельный фасад", если все методы можно перенести в RegistryImpl и не мучиться? Он же все равно о счетчиках знает, пусть их и делегирует. Да и без счетчиков он бесполезен.
вот я как раз таки унаследовал Registry от RegistryImpl и все методы Registry перенеслись в RegistryImpl (или наоборот?),
а счетчики аггрегируются, благо там всего два метода прокинуть — getCount и wasUpdated (changeCount/setCount оказались не нужны в интерфейсе).

M>Далее, зачем привязка классов к предметной области? Вот почему классы называются по тому, как они будут использоваться в коде, а не по тому, что они делают? Вот PlumesCounter. Это же не PlumesCounter. Это вообще набор абстрактных счетчиков. Вот и стоит его называть CountersGroup (или как-то похоже) и класть в общую библиотеку. А то будут счетчики перьев для птиц, счетчики чешкек для рыб и т.п.
а если мне не нужна будет эта библиотека? лучше уж 50 строк простого велосипеда, тем 250 строк реюзабельного кода который больше никогда не понадобится

однако, если мне еще раз понадобится делать счетчики — я всегда могу написать реюзабельный класс и заменить им старый не реюзабельный

Здравствуйте, Геннадий Васильев, Вы писали:

ГВ>Для начала, у тебя тут путаница из-за употребления терминов. Impl-классы как правило агрегируют по ссылке или по указателю, иначе нет смысла давать такое абстрактное название. То есть идиоматическая реализация выглядит примерно так:

Impl это от слова implementation, по русски — реализация, а не от идиомы pimpl

ГВ>Если применять наследование именно по такому сценарию — для "упрощения" делегирования, то возможна одна неприятность: если изменится контракт любого базового класса, это изменение может отразиться на пользователях CorvusRegistry. Это может быть багой, а может быть фичей, зависит от способа использования CorvusRegistry.

в данном случае при изменениях базовых классов я собираюсь менять потомка так чтобы классы использующие потомка ничего не заметили

Здравствуйте, Abyx, Вы писали:

Слишком мало контекста, есть ли другие типы реестров? Если нет, то я вообще не вижу смысла в таком разбиении. Забабахать одним классом, да и все. Это если вы привели полный контекст. А так, можно и наследованием. Реализация с агрегирванием мне не нравится.

Здравствуйте, Abyx, Вы писали:

ГВ>>Для начала, у тебя тут путаница из-за употребления терминов. Impl-классы как правило агрегируют по ссылке или по указателю, иначе нет смысла давать такое абстрактное название. То есть идиоматическая реализация выглядит примерно так:

A>Impl это от слова implementation, по русски — реализация, а не от идиомы pimpl

Тогда я не понимаю, зачем вообще городить класс реестра и к нему отдельный класс Implementation. Это всё основательно сбивает с толку тех, кто читает код.

ГВ>>Если применять наследование именно по такому сценарию — для "упрощения" делегирования, то возможна одна неприятность: если изменится контракт любого базового класса, это изменение может отразиться на пользователях CorvusRegistry. Это может быть багой, а может быть фичей, зависит от способа использования CorvusRegistry.

A>в данном случае при изменениях базовых классов я собираюсь менять потомка так чтобы классы использующие потомка ничего не заметили

Хех, если так, то лучше использовать агрегацию или защищённое (private или protected) наследование — всё равно ты собираешься изолировать пользователей от влияния базовых классов.