Как правильно инкапсулировать std::set?

StackOverflow https://stackoverflow.com/questions/1834230

  •  11-09-2019
  •  | 
  •  

Вопрос

У меня есть класс с именем Particle, который имеет std::set в качестве члена.Класс выглядит примерно так:

class Particle {
private:
    std::set<vtkIdType> cells;
    std::set<vtkIdType>::iterator ipc;

public:

    Particle() {};

    enum state {EXISTS = -1, SUCCESS = 0, ERROR = 1};

    state addCell(const vtkIdType cell);

    int numCells() { return static_cast<int>(cells.size()); }

    vtkIdType getFirstCell() { return (*(ipc = this->cells.begin()));}
    vtkIdType getNextCell() { return *(++ipc); }
    vtkIdType hasNextCell() { ++ipc; if (ipc == this->cells.end()) return false; --ipc; return true; }

    std::string getOutput();
};

Я очень недоволен тем, что getFirstCell(), getNextCell() и особенно hasNextCell(), они существуют, потому что я не хочу раскрывать сам набор.Я должен был воспользоваться проходом через ++ipc и --ipc потому что if((ipc+1) == this->cells.end()) выдает ошибку компилятора, похоже, проблема в ipc + 1.

Какой был бы хороший способ инкапсулировать набор и получить к нему доступ?Кроме того, есть ли хороший способ избавиться от getFirstCell() функция?

Заранее благодарю.

Редактировать:Код, который я опубликовал, является всего лишь примером структуры классов."Реальный" класс содержит больше наборов и других данных, которые не так важны для этого вопроса (я предполагал).

Это было полезно?

Решение

Я не уверен, почему вы не хотите раскрывать сам набор, но если это потому, что вы хотите гарантировать, что содержимое набора не может быть изменено извне class Particle просто вернись const итераторы, которые делают набор "доступным только для чтения", например

typedef std::set<vtkIdType>::const_iterator CellIterator;
CellIterator beginCell() const { return this->cells.begin(); }
CellIterator endCell() const { return this->cells.end(); }

Другие советы

Причина , по которой ipc+1 не работает разве что std::set поддерживаются только двунаправленные итераторы, которые поддерживают operator++ и operator--;для того, чтобы использовать operator+, вам нужно использовать итераторы произвольного доступа.

Одна проблема, которую я вижу в вашем дизайне, заключается в том, что ваши функции называются как средства доступа (getSuchAndSuch), но они также изменяют внутреннее состояние объекта (ipc модифицирован).Это может привести к путанице.

Одна вещь, которую вы могли бы попробовать, - это использовать несколько функций-членов, которые возвращают итераторы (a begin и end, например), и разрешите пользователям вашего класса использовать итераторы для доступа к внутреннему набору, все еще инкапсулируя реализацию set.

Вы могли бы вернуть тип итератора набора или, если вам нужно больше контроля или инкапсуляции, вы могли бы реализовать свой собственный класс iterator, который оборачивает итератор набора.

Чтобы предотвратить раскрытие set::iterator (чтобы не обещать пользователям больше, чем необходимо), вы можете создать оболочку:

class Particle::iterator
{
public:
  iterator()
  {}
  iterator &operator++()
  {
    ++InternalIterator;
    return *this;
  }
  vtkIdType &operator*() const
  {
    return *InternalIterator;
  }
  ...//other functionality required by your iterator's contract in the same way
private:
  iterator(const std::set<vtkIdType> &internalIterator)
    :InternalIterator(internalIterator)
  {}
  std::set<vtkIdType>::iterator InternalIterator;
};

Particle::iterator Particle::GetBeginCell()
{
  return iterator(cells.begin());
}
Particle::iterator Particle::GetEndCell()
{
  return iterator(cells.end());
}

Таким образом, вы избавитесь от внутреннего итератора (потому что это довольно ограничительно - иметь только один итератор) и получите возможность использовать алгоритмы из STL на итераторах Particle.

Также boost::iterator_facade может быть полезен здесь...

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

Я бы посмотрел на Particle и выяснил, может ли она обеспечить что-то значимое, помимо какого-либо способа хранения / доступа к куче ячеек.Если это действительно всего лишь простой контейнер, то вам было бы намного лучше с чем-то вроде typedef std::set<cell> Particle;, таким образом, конечный пользователь может использовать алгоритмы и тому подобное в этом наборе точно так же, как и в любом другом.Я бы написал класс для инкапсуляции только в том случае, если вы действительно можете инкапсулировать что-то значимое - т. е.если ваш Particle класс может содержать некоторые "знания" о частицах, чтобы другой код мог работать с частицей как с чем-то значимым само по себе.

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

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

Если вы хотите сохранить общую реализацию, которая у вас уже есть, но просто исключите getFirstCell(), вы могли бы инициализировать ipc внутри конструктора.Как указывалось выше, разумное использование const и четкое разграничение между средствами доступа и мутаторами прояснило бы интерфейс.Кроме того, если бы вы должны были реализовать итераторы в своем классе, то я бы рекомендовал, чтобы addcell() возвращает итератор, ссылающийся на новую ячейку, и вместо этого выдает исключение при обнаружении ошибки.

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top