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

Question

У меня есть класс с именем 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()?

Заранее спасибо.

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

Answer 1

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

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

Answer 2

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

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

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

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

Answer 3

Чтобы предотвратить выставление 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 ...

Answer 4

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

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

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

Answer 5

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

Answer 6

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