3. array soll 2d punkte aufnehmen

array soll 2d punkte aufnehmen

  • K

    grüße an alle,
    wie man dem titel schon entnehmen kann, soll mein array (muss selbst gebaut sein) 2d punkte aufnehmen können, damit ich dann auf diesem arbeiten kann.

    header:

    class point_list
{
public:
  point_list(int);
  ~point_list();

  int getSize();
  point operator[](int index){return m_data[index];}
  point_list& operator=(point_list&);
private:
  point *m_data;
  int m_size;
};

    cpp:

    point_list::point_list(int size)
{
  m_size = size;
  m_data = new point[m_size];
}

point_list::~point_list()
{
  delete [] m_data;
}

int point_list::getSize()
{
  return m_size; 
}

point_list& point_list::operator=(point_list& rhs)
{
  if(this == &rhs)  return *this;
  delete [] m_data;
  m_size = rhs.getSize();
  m_data = new point[m_size];
  for(int i = 0; i < m_size; i++)  m_data[i] = rhs[i];
  return *this;
}

    main:

    int main()
{
  point p1(2,3);
  point p2(2,4);
  point p3(2,3);
  point p4(0,0);

  point_list liste(5);

  liste[0] = p1;

    die punkte selbst sind in der klasse point implementiert.
    irgendwie schaffe ich es nicht, via index einen punkt in das array zu bringen.

    fehlermeldung vom derzeitigen code lautet:

    "make: *** [main_test] Speicherzugriffsfehler"
    "make: *** Datei >>main_test<< wird gelöscht"

    scheint so, als ob ich einen fatalen fehler machen würde.
    wäre toll wenn ihr mir (c++ - neuling) unter die arme greifen könntet.
    vielen dank
    grüße kylanysik

    0
  • D

    Das Problem liegt hier:

    point operator[](int index){return m_data[index];}

    Du gibts hier eine Kopie von m_data[index] zurück. Das du da aber etwas zuweisen kannst musst du eine Referenz zurückgeben.

    point& operator[](int index){return m_data[index];}
    0
  • A

    Kylanysik schrieb:

    ...

    Darf ich auch mal den Datentyp "point" sehen?

    Grundsätzlich habe ich aber bereits mehrere Anmerkungen:
    1. Da man gegen die Schnittstelle, nicht die Implementierung entwickeln sollte, würde ich dir grundsätzlich dazu raten auch die Parameternamen in dem Header auszuschreiben. Üblicherweise schaut man sich nur diesen an, um Code zu verwenden.
    2. "point operator[](int index)" Dir ist hoffentlich bewusst, das du hier eine Kopie eines point-Objektes zurücklieferst? Sprich: Du weist beispielsweise einer Kopie etwas zu, nicht dem Inhalt von "m_data[index]".
    3. Zuweisungsoperator und Kopierkonstruktor sind in der Regel paarweise zu implementieren! Zudem ist die übergabe einer Konstanten Referenz üblich (Du Manipulierst ja hoffentlich nicht das übergebene Objekt).
    4. Auch gut: const-correctness
    5. Ich würde Größeninformarionen als unsigned-Wert übergeben, damit schließt du übergaben negativer Werte schon aus (Die Standardbibliothek verwendet hierbei size_t).

    Ein paar Verbesserungen am Header (nachfolgend gehe ich noch auf ein paar Punkte ein):

    class point_list
{
  public:
    point_list(unsigned int size);
    point_list(point_list const & rhs); // Kopierkonstruktor
    point_list& operator=(point_list const & rhs);
    ~point_list();

    int getSize() const;
    point& operator[](unsigned int index);
    point const & operator[](unsigned int index) const;

private:
    point *m_data;
    unsigned int m_size;
};

    Erklärungen
    a) "point_list const &" ist gleichbedeutend mit "const point_list &", falls es dir besser geläufig ist (Die Gründe warum ich ersteres vorziehe, lasse ich hier erstmal außen vor)
    b) Übergaben von Konstanten Referenzen: Du möchtest bei einer Kopie oder Zuweisung ja sicher nichts am Original ändern.
    c) "int getSize() const", Konstant da diese Funktion keine Werteänderung am Objekt vornimmt. Damit kann diese Funktion auch auf konstante point_list-Objekte angewendet werden.
    d) Warum 2 mal den Indexoperator?
    => Der erste wird auf nicht-konstante point_list-Objekte angewendet, und gibt eine nicht-konstante Referenz zurück, dadurch kannst du "m_data[index]" auch etwas zuweisen.
    => Der zweite wird auf konstante point_list-Objekte angewendet, auslesen ist hier auch möglich, aber bei einer Zuweisung wird der Compiler dir einen Fehler melden.

    Jetzt deine cpp-Datei mal etwas angepasst:

    point_list::point_list(unsigned int size)
:   m_size(size),
    m_data(new point[m_size])
{
}

    Verwendung der Initialisierungsliste ist dem Konstruktorrumpf vorzuziehen (Im Konstruktorrumpf erfolgt eine Zuweisung, in der Initialisierungsliste eine Initialisierung; letzteres erlaubt auch die initialisierung von Referenzen und Konstanten).

    point_list::point_list(point_list const & rhs)
:   m_size(objekt.m_size),
    m_data(new point[m_size])
{
    // Zuweisung von Arrays erst im Konstruktorrumpf möglich
    for(unsigned int index=0; index<m_size; ++index)
        m_data[index] = objekt.m_data[index]
}

// ...

point_list& point_list::operator=(point_list const & rhs)
{
    // Es gibt bessere Varianten, unter anderen über swap-Techniken
    // dies werde ich hier jetzt nicht ausführen, dir sollte nur eines
    // bewusst sein: Im Fehlerfall kann so wie du diesen Operator
    // umgesetzt hast ein undefinierter Objektzustand eintreten
    if(this == &rhs)
        return *this; // Einrückungen lesen sich IMHO leichter
    delete [] m_data;
    m_size = rhs.getSize();
    m_data = new point[m_size];
    for(unsigned int i = 0; i < m_size; ++i) // ++i ist i++ vorzuziehen*
        m_data[i] = rhs[i];
    return *this;
}

    * Es macht bei integralen Datentypen in der Regel keinen Unterschied (da der Compiler meist wegoptimieren kann), nur ist ++i im Zweifel spätestens bei Objekten vorzuziehen (vermeidet eine zusätzliche Kopie).

    Den Rest kann ich nur mit der Kenntnis der Klasse point ansprechen.

    0
  • K

    hi, tut mir leid, dass ich erst jetzt reagieren kann, aber es hat sich leider nicht eher ergeben können.

    ich bin hin und weg von der hilfsbereitschaft, die ich hier im forum erfahren durfte.. vielen vielen dank für diese unterstützung. 🙂 (ganz dickes lob)

    ja, natürlich gibt es einblick in "point" 😉

    header:

    class point
{
public:
  point();
  point(point const&);
  point(double, double); // x, y
  ~point();

  void swap(point&);
  point& operator=(point const&);
private:
  double data_[2];
};

  double distance(point const& lhs, point const& rhs);

    cpp:

    point::point()
: data_()
{
  data_[0] = data_[1];
}

point::point(point const& a)
: data_()
{
  data_[0] = a.data_[0];
  data_[1] = a.data_[1];
}

point::point(double x, double y)
: data_()
{
  data_[0] = x;
  data_[1] = y;
}

point::~point()
{}

void
point::swap(point& rhs)
{
  std::swap_ranges(data_, data_+4, rhs.data_);
}

point&
point::operator=(point const& rhs)
{
  point tmp(rhs);

  swap(tmp);

  return *this;
}

double distance(point const& lhs, point const& rhs)
{
  return(
          std::sqrt (
                     ((lhs.data_[0] - rhs.data_[0])  * 
                      (lhs.data_[0] - rhs.data_[0])) +
                     ((lhs.data_[1] - rhs.data_[1])  *
                      (lhs.data_[1] - rhs.data_[1]))
                    )
        );
}

    ist denn der kopierkonstruktor wirklich notwendig. denn ich habe ja nur vor dem array eine menge von punkten zu geben und dann eben dieses array mithilfe einer funktion (die ich noch schreiben werde, aber schon theoretisch (aufn blatt papier) stehen habe) zwei punkte zu finden, die am nächsten zueinander liegen (mittels bruteforce). brauch ich dafür den kopierkonstruktor? -> denn mehr soll das programm dann auch nicht können.

    grüße kylanysik und nochmals dickes danke 🙂

    PS: mir ist gerade hierbei noch eine frage gekommen..
    ich habe meinen code mit deinen code zusammen geworfen, jedoch kommt nun die fehlermeldung:
    (im zuweisungsoperator)

    point_list& point_list::operator=(point_list const& rhs)
{
  if(this == &rhs)  return *this;
  delete [] m_data;
  m_size = rhs.getSize();
  m_data = new point[m_size];
  for(unsigned int i = 0; i < m_size; ++i)  m_data[i] = rhs[i]; // <- hier 
  return *this;
}

    "Fehler: Die Übergabe von >>const point_list<< als >>this<<-argument von point& point_list::operator[](unsigned int) streicht Qualifizierer"

    was verstehe ich darunter?
    ich hoffe ich nerve nicht mit meinen törichten fragen 😞

    0
  • D

    Man kann es sicher auch ohne Kopierkonstruktor hinkriegen. (Mach ihn sonst auch mal privat). Allerdings wenn er so banal ist, würde ich ihn schon zur Verfügung stellen, damit ich auch etwas per Kopie übergeben kann. Es gibt nur wenige Fälle, wo das nicht erwünscht ist und ein Punkt ist sicher keiner davon. 😉

    Ich gehe richtig in der Annahme, dass dein obiges Problem gelöst ist?

    0
  • K

    naja, so banal erscheint mir das alles noch nicht so wirklich 😞
    bin halt dabei das hier mehr oder weniger aus büchern zu lernen und bin wirklich froh drum, auch mal im forum was erklärt zu bekommen, was ich so vllt nicht so schnell verstehen würde.
    jip, dass problem mit der speicherzugriffsverletzung schon 😉

    0
  • N

    Den Kopierkonstruktor brauchst du eigentlich nur, wenn du die Semantik tiefer Kopien benötigst. Also wenn nicht nur die Member selber, sondern beispielsweise von Zeigern referenzierte Speicherbereiche kopiert werden müssen.

    In deinem Falle, wo du nur ein statisches Array als Member hast, müsstest du den Kopierkonstruktor nicht schreiben, da reicht der vom Compiler generierte.

    0
  • K

    ich bins schonwieder ^^
    es kommt hier eine fehlermeldung, worauf ich keine reaktion weiß..

    -> "terminate claaed after throwing an instance of 'std::bad_alloc'
    what(): std::bad_alloc"

    ich habe mich mal versucht darüber kundig zu machen und laut recherche liegt es an einer "new", welche eine exception wirft (also in irgendeiner weise etwas mit den speicher zu tun hat)
    wie soll ich nun darauf reagieren?

    es scheint etwas mit den konstruktor zutun haben, denn zumindest, wenn ich eben die zeile auskommentiere, welche den konstruktor in der main aufruft, bleibt die fehlermeldung aus.

    0
  • N

    Kylanysik schrieb:

    es scheint etwas mit den konstruktor zutun haben, denn zumindest, wenn ich eben die zeile auskommentiere, welche den konstruktor in der main aufruft, bleibt die fehlermeldung aus.

    Also hast du da gar kein Objekt mehr? Oder wie initialisierst du es ohne Konstruktoraufruf? Ich würde mal mit dem Debugger schrittweise durchgehen und schauen, wo die Exception fliegt. Du kannst auch mit try - und catch -Blöcken versuchen, den Fehler einzugrenzen.

    Aber ein bad_alloc , Respekt. 😉

    0
  • K

    mmh, ich komm nicht drauf, wie ich das problem behandeln soll (zudem erlauben mir meine vielen lücken nicht, das problem richtig anzupacken)

    also es liegt, soviel denke ich, habe ich bereits herausgefunden, am konstruktor:

    point_list::point_list(unsigned int size)
:
  m_size(size),
  m_data(new point[m_size])
{}

    dieser wird, wenn ich mich nicht irre durch den in der main stehenden code:

    "point_list liste(5);"

    genutzt. und eben hierbei kommt der "bad_alloc"-fehler

    gelöst(ob es wirklich gelöst ist, weiß ich nicht, aber zumindest kommt keine fehlermeldung) wäre das problem, wenn der konstruktor halt so aussähe:

    point_list::point_list(unsigned int size)
{
  m_size = size;
  m_data = new point[m_size];
}

    nunja, aber von den einem stolperstein dauert es nicht lange bis ich zum nächsten komme.

    irgendwie klappt es noch nicht, dass ich der erstellten liste via index einen punkt zuweisen kann. also inder form:

    point p1(2,3);
    liste[1] = p1;

    hierbei meint er: "Segmentation fault"

    aber wieso greife ich auf falschen speicher zu? 😞

    grüße kylanysik

    0
  • K

    auf der suche nach dem auslöser der segmentation fault - fehlermeldung, dachte ich mir, den "operator=", so wie mir empfohlen wurde, via swap() zu konstruieren. ich habe auch einige quellen dazu gefunden, jedoch wie ich eine swap richtig baue, weiß ich nicht. mein bisheriger ansatz wäre dieser hier. aber ich weiß, dass dieser falsch ist, da die swap-kandidaten andere sind, als die die rein kommen. aber wie baue ich diese denn richtig. (vielleicht wird durch eine lösung dieses problems auch die "segmentation fault" gelöst)

    void 
point_list::swap(point_list& rhs)
{
  swap(m_data, rhs.m_data);
  swap(m_size, rhs.m_size);
}

point_list& 
point_list::operator=(point_list const& rhs)
{
  point_list tmp(rhs);
  swap(tmp);
  return *this;
}

    hat einer eine idee?
    grüße kylanysik

    0
  • D

    gelöst(ob es wirklich gelöst ist, weiß ich nicht, aber zumindest kommt keine fehlermeldung) wäre das problem, wenn der konstruktor halt so aussähe:

    Hehe. Das ist ein sehr gemeiner Fehler. 😉
    Das Problem ist, dass die Initialisierungsreihenfolge nicht nach der Reihe geht, wie du es in der Initialisierungsliste hast, sondern, nach der Deklaration in der Klasse. Und somit initialisierst du den Speicher mir irgend einer Zahl, wie -89234.

    Wenn du das in den Konstruktor selbst verlagerst, wurde beides bereits initialisiert, da du aber die Werte dann per Zuweisung änderst funktioniert es (da size bereist den richtigen Wert hat).

    Also einfach das hier:

    private:
    point *m_data;
    unsigned int m_size;

    ändern:

    private:
    unsigned int m_size;
    point *m_data;

    Dann klappt das auch mit der Initialisierungsliste.

    Wie gesagt. Sehr fies. 😉

    Zeig mal den Kopierkonstruktor von point_list, respektive alle Teile, wie sie jetzt aktuell sind und mit dem Problem zu tun haben.

    0
  • K

    wow, wirklich nen fieses ding.
    darauf wäre ich nie gekommen. wie lange stehst du denn schon in c++, dass dir ein solcher fehler auffällt? ^^

    vielen dank. 🙂

    aso: (hätte ich fast vergessen)

    header:

    class point_list
{
public:
  point_list(unsigned int);
  point_list(point_list const & rhs);
  ~point_list();

  int getSize() const;
  point& operator[](unsigned int index)
    {return m_data[index];}
  point const& operator[](unsigned int index) const 
    {return m_data[index];}

  void swap(point_list&);
  point_list& operator=(point_list const&);

private:
  unsigned int m_size;
  point *m_data;
};

    cpp:

    #include "point_list.h"
#include "point.h"
#include <iostream>
#include <algorithm> //fuer swap

point_list::point_list(unsigned int size)
:
  m_size(size),
  m_data(new point[m_size])
{

}

point_list::point_list(point_list const & rhs)
:
  m_size(rhs.m_size),
  m_data(new point[m_size])
{
  for(unsigned int index=0; index < m_size; ++index)
  {
    m_data[index] = rhs.m_data[index];
  }
}

point_list::~point_list()
{
  delete [] m_data;
}

int point_list::getSize() const
{
  return m_size; 
}

void 
point_list::swap(point_list& rhs)
{
  swap(m_data, rhs.m_data);  // <-
  swap(m_size, rhs.m_size);  // <-
}

point_list& 
point_list::operator=(point_list const& rhs)
{
  point_list tmp(rhs);
  swap(tmp);
  return *this;
}

    die swap, wie sie hier ist, funktioniert bei mir nicht wirklich.
    (zumindest so, wie ich sie aus quellen her gefunden habe)
    gibt fehlermeldung, dass keine passende funktion für diesen aufruf vorhanden ist.
    da hat er ja auch recht ;), aber ich dachte das hängt mit "#include<algorithm>" zusammen und das problem würde dadurch nicht entstehen? O.o

    die swap wäre das eine problem ^^

    ich weiß aber nicht, ob das zweite was damit zu tun hat.
    wenn ich in der main:

    point p1(2,3);
point_list liste(5);
liste[0] = p1;

    soetwas möchte (und um das entgültige resultat haben zu können muss ich irgendwie via index drauf zugreifen und zuweisen), kommt nen "segmentation fault"

    grüße kylanysik

    0
  • D

    Kylanysik schrieb:

    wow, wirklich nen fieses ding.
    darauf wäre ich nie gekommen. wie lange stehst du denn schon in c++, dass dir ein solcher fehler auffällt? ^^

    vielen dank. 🙂

    Hmm. Gute Frage.. 😉 - Vlt. so 2 - 2.5 Jahre mittlerweilen.

    Mir selbst ist so ein Fehler glaube ich noch nie passiert, was aber daran liegt, dass ich eine solche "ungeordnetheit" sowieso nicht ausstehen kann und das eigl. seit dem Anfang immer in der richtigen Reihenfolge gemacht habe. Habe aber ein paar mal über den Fehler gelesen und bei dir war es ja offensichtlich, da eins zum anderen führt. 😉

    Einen namespace musst du schon auch noch angeben:

    std::swap(m_data, rhs.m_data);  // <-
  std::swap(m_size, rhs.m_size);  // <-
    0
  • K

    nun existiert lediglich eine lücke 🙂

    point p1(2,3);
point_list liste(5);
liste[0] = p1;  // <-

    wie schon geschildert funzt der aufruf nicht... wenn ich das richtig verstehe will ich speicher benutzen, der nich dafür gedacht war. oder?

    0
  • D

    Was geht denn nicht? Compilefehler, oder erst zur Laufzeit?

    0
  • K

    nunja, wenn ich es so richtig sehe, dann erst zur laufzeit.. denn die zuvor in der main aufgerufenen aufgaben werden erfüllt und erst, wenn er zu der zeile: "liste[0] = p1" kommt, zeigt er den fehler an.

    0
  • N

    Was für ein Laufzeitfehler entsteht denn? Heap Corruption? Exception? 😉

    0
  • K

    make: *** [main_test] "segmentation fault"
    make: *** Datei >>main_test<< wird gelöscht

    0
  • B

    Die swap-Funktion bei Point scheint mir fehlerhaft.

    void
point::swap(point& rhs)
{
  std::swap_ranges(data_, data_+4, rhs.data_);
}

    Data hat doch bloß 2 Elemente. Hier würden auch 2 normale std::swap Aufrufe reichen.

    0
Log in to reply