STL-Container wrappen zwecks eigener Iteratoren

[goran]

Der neue Code:

#include <iostream>
#include <string>
#include <set>

class WrapSet
{
public:
  void insert(const std::string & value)
  {
    values_.insert(value);
  }

  class iterator
  {
  public:
    iterator() {}

    iterator & operator++()
    {
      it_++;
      return * this;
    }

    WrapSet & operator*() const
    {
      return * it_;
    }

  private:
    iterator(const std::set<std::string> & it)
      : it_(it) {}

    std::set<std::string>::iterator it_;
  };

  WrapSet::iterator begin()
  {
    return iterator(values_.begin());
  }

  WrapSet::iterator end()
  {
    return iterator(values_.end());
  }

private:
  std::set<std::string> values_;
};

int main()
{
  WrapSet ws;

  ws.insert("bla");

  for(WrapSet::iterator it = ws.begin(); it != ws.end(); it++)
  {
    std::cout << *it << std::endl;
  }
}

Die Fehler:

`# g++ WrapSet.h

WrapSet.h: In member function ‘WrapSet& WrapSet::iterator::operator*() const’:

WrapSet.h:26: error: invalid initialization of reference of type ‘WrapSet&’ from expression of type ‘const std::basic_string<char, std::char_traits<char>, std::allocator<char> >’

... und noch 'ne Menge mehr`

[Nexus]

Du solltest Const-Referenzen auf deine Objekte zurückgeben. Beim std::set kann es klug sein, gar keinen schreibenden Zugriff zu erlauben. Gerade wenn ein std::string verändert wird, wird die Sortierordung von std::set mit grosser Wahrscheinlichkeit durcheinandergebracht, was in undefiniertem Verhalten resultieren kann.

Zudem solltest du für den Iterator einen Konstruktor anbieten, welcher eine Initialisierung mit Werten aus begin() und end() erlaubt. Dasselbe gilt für den Zuweisungsoperator.

[goran]

Werde ich gerne tun (const_iterator) aber zuerst benötige ich ein lauffähiges Minimalbsp.

Kann jemand sagen wo der Fehler in meinem Bsp. liegt das es nicht kompiliert?

Danke

[Nexus]

goran schrieb:

Kann jemand sagen wo der Fehler in meinem Bsp. liegt das es nicht kompiliert?

Habe ich doch schon. Gib Const-Referenzen zurück und schreibe einen Konstruktor.

goran schrieb:

Werde ich gerne tun (const_iterator) aber zuerst benötige ich ein lauffähiges Minimalbsp.

Ich habe diese Dinge gesagt, um dein Beispiel lauffähig zu machen. Das ist nicht irgendein Geschwafel von irrelevanter Theorie.

[goran]

Verstehe ich schon (oder seit deinem vorletzten Posting). Ich interpretiere "sollte" als "kann man, muss man nicht". Hättest du "musst" geschrieben hätte ich dich sofort verstanden.

Aber über eine kleine Zeile Code bzgl. des Konstruktors und des dann notwendigen operator= wäre ich dankbar. Hier bei mir kommt nur Murks raus.

[Nexus]

goran schrieb:

Ich interpretiere "sollte" als "kann man, muss man nicht". Hättest du "musst" geschrieben hätte ich dich sofort verstanden.

Stimmt, wäre vielleicht etwas klarer gewesen.

goran schrieb:

Aber über eine kleine Zeile Code bzgl. des Konstruktors und des dann notwendigen operator= wäre ich dankbar. Hier bei mir kommt nur Murks raus.

Okay. Hier hast du das ::iterator im Parametertypen vergessen.
Const-Referenz lohnt sich bei Iteratoren im Allgemeinen nicht, da diese üblicherweise sehr klein sind.

WrapSet::iterator::iterator(std::set<std::string>::iterator it)
: it_(it)
{
}

Wenn du einen impliziten Konstruktor (wie hier, ohne Schlüsselwort explicit ) hast, brauchst du eigentlich keinen Zuweisungsoperator mehr. Bei einer Zuweisung von einem std::set<std::string>::iterator würde dann ein temporäres iterator -Objekt erstellt werden, und dieses könnte der Default-Zuweisungsoperator zuweisen.

Aber fang am besten gar nicht erst an, einen Schreibzugriffs-Iterator bereitzustellen. Wechsle gleich auf const_iterator , dann brauchst du auch weniger überladene Methoden und einiges wird einfacher und vor allem sicherer.

[unskilled]

(typename std::set<std::string>::iterator it)

ich will wieder stackende [ c] und [ b]`s

bb

[goran]

Danke sehr für die Hilfe.

Bis hierhin sieht meine WrapSet.c so aus:

#include <iostream>
#include <string>
#include <set>

class WrapSet
{
public:
  class const_iterator
  {
  public:
    const_iterator() {}
    const_iterator(std::set<std::string>::const_iterator internal_iterator)
      : internal_iterator_(internal_iterator) {}

    const_iterator & operator++(int i)
    {
      ++internal_iterator_;
      return * this;
    }

    const_iterator & operator=(const const_iterator ci)
    {
      internal_iterator_ = ci.internal_iterator_;
    }

    const std::string & operator*()
    {
      return *internal_iterator_;
    }

    bool operator!=(const const_iterator ci)
    {
      if(internal_iterator_ == ci.internal_iterator_)
        return true;
      else
        return false;
    }

  private:
    std::set<std::string>::const_iterator internal_iterator_;
  };

  void insert(const std::string & value)
  {
    values_.insert(value);
  }

  const_iterator begin()
  {
    return const_iterator(values_.begin());
  }

  const_iterator end()
  {
    return const_iterator(values_.end());
  }

private:
  std::set<std::string> values_;
};

int main()
{
  WrapSet ws;

  ws.insert("bla");

  for(WrapSet::const_iterator i=ws.begin(); i!=ws.end(); i++)
    std::cout << *i << std::endl;
}

Natürlich habe ich wieder Fragen dazu:

Zeile 12:
In meinem Ausgangs-Bsp. war der Konstruktor in dieser Zeile privat. War das falsch? Ich habe ihn nach public verschoben.

Zeile 15:
Die Rückgabe ist eine Referenz. Ist es besser ohne Referenz? Und weiter, wieso benötigt der operator++ ein int übergeben?

Zeile 21:
Wie in Zeile 15. Ist keine Referenz besser?

Zeile 48+53:
Wie in Zeilen 21 und 15 nur umgekehrt. Diesmal keine Referenzen. Was ist besser?

[unskilled]

class WrapSet
{
public:
  class const_iterator
  {
  public:
    const_iterator()
    {}

    const_iterator(std::set<std::string>::const_iterator val)
    : internal_iterator_(val)
    {}

    const_iterator& operator++()
    {
      ++internal_iterator_;
    }

    const_iterator operator++(int i) 
    {
      const_iterator nrvo(*this);
      ++*this; 
      return nrvo;
    }

    const std::string& operator*()
    {
      return *internal_iterator_;
    }

  private: 
    std::set<std::string>::const_iterator internal_iterator_; 
  }; 

  void insert(const std::string& value)
  {
    values_.insert(value);
  }

  const_iterator begin()
  {
    return values_.begin();
  }

  const_iterator end()
  {
    return values_.end();
  }

private:
  std::set<std::string> values_;
};

so sollte es stimmen...

Zeile 12:
In meinem Ausgangs-Bsp. war der Konstruktor in dieser Zeile privat. War das falsch? Ich habe ihn nach public verschoben.

Jopp - private ctor an sich ist nciht falsch.
ich würd so ne ctor`s private machen und die umschließende klasse als friend eintragen

Zeile 15:
Die Rückgabe ist eine Referenz. Ist es besser ohne Referenz? Und weiter, wieso benötigt der operator++ ein int übergeben?

bei op++(int) geht es nicht, eine referenz zurückzugeben

Zeile 21:
Wie in Zeile 15. Ist keine Referenz besser?

ne, hier wäre die referenz richtig gewesen
es hätte aber noch das return *this; gefehlt.
nichts anderes macht aber der vom compiler automatisch generierte op= schon - also ersparen wir uns die tipp-arbeit doch...^^

Zeile 48+53:
Wie in Zeilen 21 und 15 nur umgekehrt. Diesmal keine Referenzen. Was ist besser?

referenzen auf lokale objekte gehen ins auge - hier musst du per value zurückgeben...

bb :xmas1:

[goran]

Jo, besten Dank. Jetzt läuft er.

Eine (oder zwei) Frage(n) noch.

Dein op++(int) benutzt 'ne Hilfsvar. Warum?

Und was ist das überhaupt, ein op++(int). Gibt's da ein Stichwort? Dann schau ich gerne selber nach.

Danke und Gruß

[drakon]

Das ist für die Unterscheidung von Pre/Postinkrement.

Hier sollte das im entsprechenden Abschnitt erklärt sein:
http://magazin.c-plusplus.net/artikel/�berladung von Operatoren in CPlusPlus (Teil 1)

[goran]

So, jetzt habe ich eine SetWrap-Klasse die templates unterstützt.

Zu Anfang lief es noch nicht und in diesem Beitrag waren die Fehler samt meinen Fragen beschrieben. Um den Thread aber nicht unnötig aufzublähen (wg. des Quellcodes) habe ich mich entschlossen diesen zu editieren.

Der Code:

#include <iostream>
#include <string>
#include <set>

template <class T>
class WrapSet
{
public:
  class const_iterator
  {
  public:
    const_iterator();

    const_iterator & operator++();
    const_iterator operator++(int);
    const T & operator*();
    bool operator!=(const const_iterator ci);

    friend class WrapSet;

  private:
    const_iterator(typename std::set<T>::const_iterator cit);

    typename std::set<T>::const_iterator cit_;
  };

  void insert(const T & value);
  const_iterator begin();
  const_iterator end();

private:
  std::set<T> values_;
};

template <class T>
WrapSet<T>::const_iterator::const_iterator()
{
}

template <class T>
WrapSet<T>::const_iterator::const_iterator(typename std::set<T>::const_iterator cit)
  : cit_(cit)
{
}

template <class T>
typename WrapSet<T>::const_iterator &
WrapSet<T>::const_iterator::operator++()
{
  ++cit_;
  return * this;
}

template <class T>
typename WrapSet<T>::const_iterator
WrapSet<T>::const_iterator::operator++(int)
{
  const_iterator tmp(*this);
  ++(*this);
  return tmp;
}

template <class T>
const T &
WrapSet<T>::const_iterator::operator*()
{
  return * cit_;
}

template <class T>
bool
WrapSet<T>::const_iterator::operator!=(const WrapSet<T>::const_iterator ci)
{
  return !(cit_ == ci.cit_);
}

template <class T>
void
WrapSet<T>::insert(const T & value)
{
  values_.insert(value);
}

template <class T>
typename WrapSet<T>::const_iterator
WrapSet<T>::begin()
{
  return WrapSet<T>::const_iterator(values_.begin());
}

template <class T>
typename WrapSet<T>::const_iterator
WrapSet<T>::end()
{
  return WrapSet<T>::const_iterator(values_.end());
}

int main()
{
  WrapSet<std::string> ws;

  ws.insert("bla");

  for(WrapSet<std::string>::const_iterator i=ws.begin(); i!=ws.end(); i++)
    std::cout << *i << std::endl;
}

Eine Frage zu typename habe ich noch. Nur aufgrund von try & error habe ich typename in den Zeilen 22, 24, 41, 47, 55, 85 und 92 genutzt. Was ist der Grund das typename benötigt wird?

Danke und Gruß

[Th69]

Ohne 'typename ' würde der Compiler bei

std::set<T>::const_iterator

'const_iterator' als Name einer Klassenvariablen annehmen, d.h. typename braucht man immer bei verschachtelten Typ-Zugriffen mit Templates.

[unskilled]

speziell op!= würde ich nicht als member deklarieren.
die definition ist auch höchst eigenartig

friend class WrapSet;
das class ist imho unnötig - und ich würd das entweder ganz oben oder ganz unten hinschreiben, aber nicht unter die fkt. mischen

const T & operator*();
für const T& gibt es schon fertige typedefs...
(das gleiche bei void insert(const T & value); )

bei der implementierung wäre ein wenig ordnung wünschenswert ;o)
(und in echt würd ich das auf jeden fall in ne *.impl datei auslagern, damit der header wirklich nur für die dokumentation dient)

const_iterator & operator++(); , return * this; , ...
die leerzeichen zwischen typ und & bzw * find ich eher verwirrend, aber wenn du damit besser zurecht kommst, kannst du es auch so lassen...

typename std::set<T>::const_iterator
dafür könnte man btw noch nen (privates) typedef á la internal_iter anlegen...

for(WrapSet<std::string>::const_iterator i=ws.begin(); i!=ws.end(); i++)
das man nicht jedes mal wieder end() aufruft, ist dir bekannt und du hast es nur hier so gemacht!?
ich würd so was hier schreiben:
for(WrapSet<std::string>::const_iterator i(ws.begin()), e(ws.end()); i != e; ++i)

(die ausgabe geht mittels std::copy und std::ostream_iterator auch einfacher(oder zumindest kürzer^^), aber das soll hier nicht das thema sein^^)

bb