Wie ein std :: einzukapseln richtig eingestellt?

Question

Ich habe eine Klasse mit dem Namen Teilchen, die eine std :: hat gesetzt als Mitglied. Die Klasse sieht wie folgt aus:

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();
};

Ich bin sehr unzufrieden mit der getFirstCell(), getNextCell() und besonders hasNextCell(), existieren sie, weil ich mag nicht das Gerät selbst belichten. Ich hatte den Weg durch ++ipc und --ipc zu verwenden, da if((ipc+1) == this->cells.end()) einen Compiler-Fehler gibt, ipc + 1 scheint das Problem zu sein.

Was wäre ein guter Weg, um einen Satz zu kapseln und darauf zugreifen? Außerdem gibt es eine schöne Art und Weise der getFirstCell() Funktion loswerden?

Vielen Dank im Voraus.

Edit: Der Code, den ich geschrieben ist nur ein Beispiel für die Klassen-Struktur. Die „echte“ Klasse enthält mehr Sätze und andere Daten, die nicht so wichtig für diese Frage (I angenommen).

Answer 1

Ich bin mir nicht sicher, warum Sie wollen nicht das Gerät selbst belichten, aber wenn es ist, weil Sie, dass der Inhalt des Satzes sicherstellen will, kann nicht außerhalb class Particle verändert wird nur zurückkehren const Iteratoren, die den Satz „macht Read- nur“, zB

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

Answer 2

Der Grund, dass ipc+1 nicht funktioniert, ist, dass std::set nur bidirektionale Iteratoren unterstützt, die operator++ und operator-- unterstützen; um operator+ zu verwenden, müssen Sie Random Access Iteratoren verwenden.

Eine Sorge, die ich mit Ihrem Design zu sehen ist, dass Ihre Funktionen benannt sind wie Accessoren (getSuchAndSuch), aber sie verändern auch den internen Zustand des Objekts (ipc wird modifiziert). Dies könnte zu Verwirrung führen.

Eine Sache, die Sie versuchen könnten, wären ein paar Member-Funktionen zu verwenden, die Iteratoren (ein begin und end, zum Beispiel) zurück und ermöglichen Benutzer Ihrer Klasse Iteratoren zu verwenden, um den internen Satz zuzugreifen, während immer noch die Menge einkapseln Umsetzung.

Sie können die Set Iteratortyp zurückkehren oder wenn Sie mehr Kontrolle oder Verkapselung möchten, können Sie Ihre eigene Iterator-Klasse implementieren, die die Set-Iterator hüllt.

Answer 3

Belichtungs Satz Um zu verhindern :: iterator (nicht für Nutzer versprechen mehr als erforderlich) Sie einen Wrapper erstellen:

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());
}

So erhalten Sie von internem Iterator befreien (weil es sehr restriktiv ist in der Lage sein, nur eine Iterator zu haben) und erhält Fähigkeit Algorithmen von STL zu verwenden, um auf Partikel Iteratoren.

Auch boost :: iterator_facade nützlich sein kann, hier ...

Answer 4

Die Frage ist wirklich, was Sie versuchen hier zu erreichen. Gerade jetzt, Ihre Klasse scheint (zumindest für mich) mehr Schaden als Gutes zu tun -. Es macht mit dem Inhalt des Satzes schwieriger statt einfacher arbeiten

Ich würde bei Particle schauen und herauszufinden, ob es etwas Sinnvolles jenseits irgendeine Weise der Speicherung / Zugriff auf eine Reihe von Zellen zur Verfügung stellen kann. Wenn es wirklich nur ein einfacher Behälter ist, dann würden Sie viel besser mit so etwas wie typedef std::set<cell> Particle; sein, so dass der Endbenutzer-Algorithmen und so auf diesen Satz wie sie können jede andere verwenden können. Ich würde nur eine Klasse schreiben, dass verkapseln, wenn Sie wirklich sinnvoll etwas einkapseln können -. Dh wenn Ihr Particle Klasse einige „Wissen“ über Partikel so einen anderen Code mit einem Teilchen als ein Ding kann arbeiten, die sinnvoll ist in sich selbst verkörpern kann

Im Moment Ihre Particle ist nichts anderes als ein Behälter - und es sieht nicht wie ein besonders guter Behälter entweder. Sofern Sie nicht wirklich etwas hinzufügen können, könnten Sie besser dran, nur sein mit, was schon da ist.

Answer 5

, was Sie zeigen, tut nichts, außer den drei Getter. die Menge einkapseln durch die Operationen zu machen, dass diese Getter Teil der Particle-Klasse verwenden würde, dann werden Sie nicht die Getter brauchen überhaupt. voila, verkapselte

Answer 6

Wenn Sie die allgemeine Durchführung behalten möchten Sie bereits haben, aber einfach getFirstCell() zu beseitigen, könnten Sie ipc im Konstruktor initialisieren. Wie oben erwähnt, vernünftige Nutzung const und klare Differenzierung zwischen Zugriffs- und Mutatoren würde die Schnittstelle klären. Außerdem, wenn Sie auf Ihre Klasse Iteratoren implementieren wäre, dann würde ich das empfehlen addcell() einen Iterator Rückkehr in die neue Zelle verweisen, und stattdessen eine Ausnahme nach der Begegnung einen Fehler aus.