Это хороший (правильный) способ инкапсулировать коллекцию? - PullRequest
3 голосов
/ 20 сентября 2008
class MyContainedClass {
};

class MyClass {
public:
  MyContainedClass * getElement() {
    // ...
    std::list<MyContainedClass>::iterator it = ... // retrieve somehow
    return &(*it);
  }
  // other methods
private:
  std::list<MyContainedClass> m_contained;
};

Хотя msdn говорит, что std::list не должно выполнять перемещение элементов при удалении или вставке, это хороший и распространенный способ вернуть указатель на элемент списка?

PS: я знаю, что могу использовать коллекцию указателей (и мне придется delete элементов в деструкторе), коллекцию общих указателей (что мне не нравится) и т. Д.

Ответы [ 9 ]

1 голос
/ 20 сентября 2008

Подумайте хороший и жесткий о том, что вы действительно хотите MyClass для. Я заметил, что некоторые программисты пишут обертки для своих коллекций просто по привычке, независимо от того, имеют ли они какие-то особые потребности, помимо тех, которые встречаются в стандартных коллекциях STL. Если это ваша ситуация, то typedef std::list<MyContainedClass> MyClass и покончим с этим.

Если у вас действительно есть операции, которые вы собираетесь реализовать в MyClass, то успех вашей инкапсуляции будет зависеть больше от интерфейса, который вы им предоставляете, чем от того, как вы предоставляете доступ к базовому списку.

Не в обиду, но ... Из-за предоставленной вами ограниченной информации она пахнет так, как будто вы пытаетесь: раскрыть внутренние данные, потому что вы не можете понять, как реализовать свои операции. Код клиента требует в MyClass ... или, возможно, потому что вы даже не знаете , какие операции потребует ваш код клиента. Это классическая проблема с попыткой написать низкоуровневый код перед высокоуровневым кодом, который в этом нуждается; вы знаете, с какими данными вы будете работать, но на самом деле еще точно не определили, что вы будете делать с ними, поэтому вы пишете структуру класса, которая предоставляет необработанные данные вплоть до самого верха. Вы бы хорошо переосмыслили свою стратегию здесь.


@ cos :

Конечно, я инкапсулирую MyContainedClass не только ради инкапсуляции. Давайте возьмем больше конкретный пример:

Ваш пример мало помогает, чтобы развеять мой страх, что вы пишете свои контейнеры, прежде чем узнаете, для чего они будут использоваться. Ваш пример контейнера-контейнера - Document - имеет в общей сложности три метода: NewParagraph(), DeleteParagraph() и GetParagraph(), каждый из которых работает с содержащейся коллекцией (std::list), и все из которых тесно отражает операции, которые std::list предоставляет «из коробки». Document инкапсулирует std :: list в том смысле, что клиентам не нужно знать о его использовании в реализации ... но реально это немного больше, чем фасад - так как вы предоставляете необработанные указатели клиентов на объекты, хранящиеся в список, клиент по-прежнему неявно привязан к реализации.

Если мы помещаем объекты (не указатели) в контейнер они будут уничтожены автоматически (что хорошо).

Хорошо это или плохо, зависит от потребностей вашей системы. Что означает эта реализация, просто: документ владеет Paragraph s, и когда Paragraph удаляется из документа, любые указатели на него немедленно становятся недействительными. Это означает, что вы должны быть очень осторожными при реализации чего-то вроде:

другие объекты, кроме использования коллекций абзацы, но не владейте ими.

Теперь у вас есть проблема. Ваш объект, ParagraphSelectionDialog, имеет список указателей на Paragraph объектов, принадлежащих Document. Если вы не будете осторожны в координации этих двух объектов, Document - или другой клиент через Document - может сделать недействительными некоторые или все указатели, содержащиеся в экземпляре ParagraphSelectionDialog! Нет простого способа поймать это - указатель на действительный Paragraph выглядит так же, как указатель на освобожденный Paragraph, и может даже в конечном итоге указать на действительный - но другой - Paragraph экземпляр! Поскольку клиентам разрешено и даже ожидается, что они могут сохранять и разыменовывать эти указатели, Document теряет контроль над ними, как только они возвращаются из открытого метода, даже если он сохраняет владение объектами Paragraph.

Это ... плохо. В результате вы получаете неполную, поверхностную инкапсуляцию, утечку абстракции, и в некотором смысле это хуже, чем отсутствие абстракции вообще. Поскольку вы скрываете реализацию, ваши клиенты не имеют представления о времени жизни объектов, на которые указывает ваш интерфейс. Вам, вероятно, повезет в большинстве случаев, так как большинство операций std::list не делают недействительными ссылки на элементы, которые они не изменяют. И все будет хорошо ... до тех пор, пока неправильный Paragraph не будет удален, и вы застрянете с задачей проследить через стек вызовов, ища клиента, который удерживал этот указатель немного слишком долго.

Исправление достаточно простое: возвращает значения или объекты, которые могут храниться столько, сколько нужно, и проверяется перед использованием. Это может быть что-то такое же простое, как порядковый номер или значение идентификатора, которое должно быть передано Document в обмен на полезную ссылку, или такое сложное, как умный указатель со счетчиком ссылок или слабый указатель ... это действительно зависит от конкретного потребности ваших клиентов. Сначала укажите клиентский код, а затем введите Document для обслуживания.

1 голос
/ 23 сентября 2008

Легкий путь

@ cos, Для примера, который вы показали, я бы сказал, что самым простым способом создания этой системы на C ++ было бы не беспокоиться о подсчете ссылок. Все, что вам нужно сделать, это убедиться, что поток программы сначала уничтожает объекты (представления), которые содержат прямые ссылки на объекты (абзацы) в коллекции, прежде чем корневой документ будет уничтожен.

Трудный путь

Однако, если вы все еще хотите контролировать время жизни путем отслеживания ссылок, вам, возможно, придется держать ссылки глубже в иерархии, чтобы объекты Paragraph содержали обратные ссылки на корневой объект Document, так что только когда последний объект абзаца будет уничтожен объект документа разрушается.

Кроме того, ссылки на абзацы при использовании внутри класса Views и при передаче другим классам также должны передаваться как интерфейсы с подсчетом ссылок.

Toughness

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

Альтернативные платформы

Этот вид инструментов может быть проще выполнить на платформе, которая поддерживает и продвигает такой стиль программирования, как .NET или Java.

Вам все еще нужно беспокоиться о памяти

Даже с такой платформой вам все равно придется обеспечивать правильную разыменовку ваших объектов. Еще выдающиеся ссылки могут съесть вашу память в мгновение ока. Итак, вы видите, что подсчет ссылок не является панацеей от хороших практик программирования, хотя он помогает избежать множества проверок и очисток от ошибок, что при применении всей системы значительно облегчает задачу программистов.

Рекомендация

Тем не менее, возвращаясь к вашему первоначальному вопросу, который вызвал все сомнения в подсчете ссылок - можно ли выставлять ваши объекты непосредственно из коллекции?

Программы не могут существовать, если все классы / все части программы действительно взаимозависимы друг от друга. Нет, это было бы невозможно, поскольку программа - это текущее проявление взаимодействия ваших классов / модулей. Идеальный дизайн может только минимизировать зависимости и не удалять их полностью.

Так что, по моему мнению, да, это неплохая практика - выставлять ссылки на объекты из вашей коллекции, на другие объекты, которые должны с ними работать, при условии, что вы делаете это в здравом уме

  1. Убедитесь, что только несколько классов / частей вашей программы могут получить такие ссылки для обеспечения минимальной взаимозависимости.

  2. Убедитесь, что переданные ссылки / указатели являются интерфейсами, а не конкретными объектами, чтобы избежать взаимозависимости между конкретными классами.

  3. Убедитесь, что ссылки не передаются дальше вглубь программы.

  4. Убедитесь, что логика программы заботится об уничтожении зависимых объектов, прежде чем очищать реальные объекты, которые удовлетворяют этим ссылкам.

1 голос
/ 20 сентября 2008

Скотт Мейерс в своей книге Эффективный STL: 50 конкретных способов улучшить использование стандартной библиотеки шаблонов говорит, что просто не стоит пытаться инкапсулировать ваши контейнеры, поскольку ни один из них не может полностью заменить другой.

1 голос
/ 20 сентября 2008

Это зависит ...

Это зависит от того, насколько инкапсулирован ваш класс, и что вы хотите скрыть или показать.

Код, который я вижу, мне подходит. Вы правы в том, что данные и итераторы std :: list не будут аннулированы в случае изменения / удаления других данных / итератора.

Теперь возврат указателя скрыл бы тот факт, что вы используете std :: list в качестве внутреннего контейнера, и не позволил бы пользователю перемещаться по его списку. Возвращение итератора предоставило бы больше свободы для навигации по этому списку для пользователей класса, но они «знали бы», что они обращаются к контейнеру STL.

Думаю, это твой выбор.

Обратите внимание, что если это == std :: list <>. End (), то у вас будут проблемы с этим кодом, но я думаю, вы уже это знаете, и это не является предметом этого обсуждения.

Тем не менее, есть альтернатива, которую я суммирую ниже:

Использование const поможет ...

Тот факт, что вы возвращаете неконстантный указатель, позволяет пользователю вашего объекта молча изменять любой класс MyContainedClass, который он / она может получить, не говоря о вашем объекте.

Вместо этого или возвращая указатель, вы можете вернуть указатель const (и суффикс вашего метода с помощью const), чтобы пользователь не мог изменять данные в списке без использования утвержденного вами средства доступа (что-то вроде setElement?) .

  const MyContainedClass * getElement() const {
    // ...
    std::list<MyContainedClass>::const_iterator it = ... // retrieve somehow
    return &(*it);
  }

Это несколько увеличит инкапсуляцию.

А как насчет ссылки?

Если ваш метод не может завершиться с ошибкой (т. Е. Он всегда возвращает действительный указатель), то вам следует рассмотреть возможность возврата ссылки вместо указателя. Что-то вроде:

  const MyContainedClass & getElement() const {
    // ...
    std::list<MyContainedClass>::const_iterator it = ... // retrieve somehow
    return *it;
  }

Это не имеет ничего общего с инкапсуляцией, хотя .. : -Р

Использование итератора?

Почему бы не вернуть итератор вместо указателя? Если для вас навигация по списку вверх и вниз - это нормально, то итератор будет лучше, чем указатель, и используется в основном таким же образом.

Сделайте итератор const_iterator, если вы хотите избежать изменения данных пользователем.

  std::list<MyContainedClass>::const_iterator getElement() const {
    // ...
    std::list<MyContainedClass>::const_iterator it = ... // retrieve somehow
    return it;
  }

Хорошей стороной будет то, что пользователь сможет перемещаться по списку. Плохая сторона в том, что пользователь будет знать, что это std :: list, поэтому ...

1 голос
/ 20 сентября 2008

В общем, если ваш «содержащийся класс» действительно содержится в вашем «MyClass», то MyClass не должен позволять посторонним касаться своего личного содержимого.

Итак, MyClass должен предоставлять методы для управления содержащимися объектами класса, а не возвращать на них указатели. Так, например, метод, такой как «увеличивать значение сотого содержавшегося объекта», а не «вот указатель на сотый содержавшийся объект, делать с ним, как вы хотите».

1 голос
/ 20 сентября 2008

Я не вижу смысла в инкапсуляции этого, но это может быть только я. В любом случае, возвращение ссылки вместо указателя имеет для меня гораздо больше смысла.

0 голосов
/ 20 сентября 2008

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

Кроме того, большинство контейнеров поддерживают несколько схожих операций, таких как начало и конец и т. Д. Поэтому изменение типа контейнера в вашем коде должно быть довольно простым, если вы измените контейнер. например, вектор для удаления или отображение на hash_map и т. д.

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

Было бы проще, если бы классы ST1, предназначенные для наследования, но ради эффективности, было решено не делать этого. Google для "унаследовать от классов STL" для большего количества мыслей по этому поводу.

0 голосов
/ 20 сентября 2008

std :: list не лишит законной силы никакие итераторы, указатели или ссылки, когда вы добавляете или удаляете объекты из списка (очевидно, кроме тех, которые указывают на удаляемый элемент), поэтому использование списка таким способом не будет сломать.

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

  const MyContainedClass * getElement() const {
    // ...
    std::list<MyContainedClass>::const_iterator it = ... // retrieve somehow
    return &(*it);
  }

может быть лучше, или если вы всегда возвращаете действительный объект MyContainedClass, тогда вы можете использовать

    const MyContainedClass& getElement() const {
    // ...
    std::list<MyContainedClass>::const_iterator it = ... // retrieve somehow
    return *it;
  }

чтобы код вызова не справлялся с указателями NULL.

0 голосов
/ 20 сентября 2008

Я думаю, что большая проблема в том, что вы скрываете тип коллекции, поэтому даже если вы используете коллекцию, которая не перемещает элементы, вы можете передумать в будущем. Внешне это не видно, поэтому я бы сказал, что это плохая идея.

...