gelöst: Problem bei doppelter Zeigerarithmetik

[EOutOfResources]

So ein Quatsch! Wirf die Klasse " Zeile " weg und gestalte Matrix um:

• Nutze intern einen std::vector .
• Nutze für Indexangaben bitte std::size_t .
• Spare an assert , nutze Exceptions.
• In der Hauptdatei solltest du den Header inkludieren.
• Mach keine Funktion print dafür einen Left-Shift-Operator mit ostream .
• Ich würde den Rückgabewert von Operatoren die keine Referenz auf sich selbst zurückgeben, const machen.
• Matrix::transpose sollte doch sich selbst transponieren und nicht eine Kopie zurrückgeben.
• Mache alle Funktionen, welche *this nicht verändern const .

[SeppJ]

EOutOfResources schrieb:

[*]Spare an assert , nutze Exceptions.

Hier sind asserts ja wohl mehr als angebracht.

[*] Matrix::transpose sollte doch sich selbst transponieren und nicht eine Kopie zurrückgeben.

Das klingt eher unpraktisch für eine Matrixklasse.

[EOutOfResources]

SeppJ schrieb:

Das klingt eher unpraktisch für eine Matrixklasse.

Wieso sollte die Funktion nicht das Objekt selbst verändern? Wenn man eine Kopie will soll er die Funktion frei implementieren.

[Nexus]

EOutOfResources schrieb:

Ich würde den Rückgabewert von Operatoren die keine Referenz auf sich selbst zurückgeben, const machen.

Ich nicht. Hat nämlich keine wirklichen Vorteile

EOutOfResources schrieb:

Wenn man eine Kopie will soll er die Funktion frei implementieren.

Kopie/In-Place und Member/freie Funktion müssen nicht voneinander abhängen.

Fakt ist, dass transpose() in beiden Fällen über das öffentliche Interface der Klasse implementiert werden kann, was eher für freie Funktion spricht.

[SeppJ]

EOutOfResources schrieb:

SeppJ schrieb:

Das klingt eher unpraktisch für eine Matrixklasse.

Wieso sollte die Funktion nicht das Objekt selbst verändern? Wenn man eine Kopie will soll er die Funktion frei implementieren.

Man muss nicht immer nur die Technik angucken, sondern auch die Semantik. Bei einer Matrix will man meistens Zugriff auf beides gleichzeitig haben, die Transponierte und die Nicht-Transponierte. Da ist es sehr unpraktisch, wenn diese Aktion die Matrix selbst jedes mal ändert. Und eine Kopie möchte man in der Regel auch nicht, denn die Transposition ist trivial, die Kopie in der Regel teuer. Insofern ist die jetzige Lösung auch nicht gerade günstig.
Ich würde ein Proxyobjekt zurückgeben, welches erst einmal nur die Zugriffe umlenkt. Und erst wenn man mit diesem Objekt eine neue Matrix erstellt, wird kopiert.

[Gast]

EOutOfResources schrieb:

[*]Nutzen für Indexangaben bitte std::size_t .

Wieso? Wieso sollte man kein int verwenden?

[Nexus]

SeppJ schrieb:

Ich würde ein Proxyobjekt zurückgeben, welches erst einmal nur die Zugriffe umlenkt. Und erst wenn man mit diesem Objekt eine neue Matrix erstellt, wird kopiert.

Copy-On-Write ist problematisch, solange man operator[] mit Schreib- und Lesezugriff hat. Dann werden nämlich auch Nicht-Änderungen als Änderungen angesehen. Eine Möglichkeit wäre separate Getter und Setter, was dann die Syntax aber weniger schön macht.

Andererseits kann man auch

void   transpose(Matrix& matrix);
Matrix transpose_copy(const Matrix& matrix);

anbieten, wobei die zweite Funktion auf die erste zugreift. Diese kann dann immer noch Indizes tauschen.

[SeppJ]

Gugelmoser schrieb:

EOutOfResources schrieb:

[*]Nutzen für Indexangaben bitte std::size_t .

Wieso? Wieso sollte man kein int verwenden?

Prinzipiell ist das "richtiger". Die Größenangaben bei den Containern sind alle in size_t, welcher in der Regel einen größeren Wertebereich als int haben wird, man reizt seine Container als nicht aus. Oder eventuell ist es auch umgekehrt. Außerdem bekommt man nicht dauernd Warnungen von wegen Vergleich von unsigned und signed. Und es ist portabel, falls size_t mal etwas exotisches sein sollte.
Der Nachteil ist, dass man Programmierfehler schwieriger diagnostizieren kann, weil ein negativer Index die gleiche Auswirkung hat wie ein zu großer Index.

Das alles sind aber ziemliche Feinheiten ohne große Relevanz. Daher kann man aber auch gleich den dafür vorgesehenen Typ nehmen, dann ist wenigstens sichergestellt, dass man keine bösen Überraschungen auf exotischen Systemen erlebt.

[Kiigass]

CStoll schrieb:

Der Fehler liegt vermutlich in Zeile 30 der matrix.hpp - du hast dort einen Zeiger auf (ein Array von) Zeiger auf Spalten definiert, also mußt du den später auch zweifach dereferenzieren, um auf die einzelnen Zeilen zugreifen zu können - also (*m.mat[i])[j] .

PS: Wer fängt denn C++ mit sowas an? Ein vernünftiger C++ Entwickler würde sowas mit STL-Containern oder std::valarray zusammenbauen.

danke für die Antwort. Ich bin noch nicht dazu gekommen die Lösung von dir auszuprobieren, aber leider wäre sie laut Aufgabenstellung auch nicht zulässig. Ganz nebenbei stimme ich dir was die Sinnhaftigkeit angeht natürlich zu.

@SeppJ: Mein Problem lässt sich am leichtesten verdeutlichen, wenn man die Zeilen reaktiviert und versucht es zu kompilieren.

[CStoll]

Kiigass schrieb:

danke für die Antwort. Ich bin noch nicht dazu gekommen die Lösung von dir auszuprobieren, aber leider wäre sie laut Aufgabenstellung auch nicht zulässig.

Was wäre nicht zulässig? vernünftige Container verwenden? Oder die zusätzliche Deferenzierung?

Um letztere wirst du nicht herumkommen, wenn du den jetzigen Aufbau beibehalten willst/mußt:

m.mat ist ein Zeile** - ein Zeiger auf ein Zeile*-Array
m.mat[i] ist ein Element dieses Arrays, also ein Zeiger auf ein Zeile
*m.mat[i] ist das Objekt hinter diesem Zeiger
(*m.mat[i])[j] ruft die op[]-Methode der Zeile auf

[Kiigass]

CStoll schrieb:

Kiigass schrieb:

danke für die Antwort. Ich bin noch nicht dazu gekommen die Lösung von dir auszuprobieren, aber leider wäre sie laut Aufgabenstellung auch nicht zulässig.

Was wäre nicht zulässig? vernünftige Container verwenden? Oder die zusätzliche Deferenzierung?

Um letztere wirst du nicht herumkommen, wenn du den jetzigen Aufbau beibehalten willst/mußt:

m.mat ist ein Zeile** - ein Zeiger auf ein Zeile*-Array
m.mat[i] ist ein Element dieses Arrays, also ein Zeiger auf ein Zeile
*m.mat[i] ist das Objekt hinter diesem Zeiger
(*m.mat[i])[j] ruft die op[]-Methode der Zeile auf

Ja die zusätzlich Dereferenzierung bzw. genauer: Die Syntaxänderung zum Aufruf der Daten ist unzulässig. Laut Aufgabenstellung muss die Syntax sein: "m[i][j]".
Allerdings gibt es dafür ja auch den überladenen operator[] der eigentlich das Objekt zurück geben sollte (also bei der Klasse Matrix soll er ein Objekt vom Typ Zeile zurück geben und bei der Klasse Zeile einen Integer). Vllt steckt auch da das Problem.

[CStoll]

Kiigass schrieb:

Ja die zusätzlich Dereferenzierung bzw. genauer: Die Syntaxänderung zum Aufruf der Daten ist unzulässig. Laut Aufgabenstellung muss die Syntax sein: "m[i][j]".
Allerdings gibt es dafür ja auch den überladenen operator[] der eigentlich das Objekt zurück geben sollte (also bei der Klasse Matrix soll er ein Objekt vom Typ Zeile zurück geben und bei der Klasse Zeile einen Integer). Vllt steckt auch da das Problem.

Warum verwendest du den dann nicht in deinen Funktionen? Das Teil sollte nicht die Aufrufe von außen ersetzen, sondern deine missglückten Aufrufe ma.mat[i][j] in den Operatoren.

[SeppJ]

Nexus schrieb:

SeppJ schrieb:

Ich würde ein Proxyobjekt zurückgeben, welches erst einmal nur die Zugriffe umlenkt. Und erst wenn man mit diesem Objekt eine neue Matrix erstellt, wird kopiert.

Copy-On-Write ist problematisch, solange man operator[] mit Schreib- und Lesezugriff hat. Dann werden nämlich auch Nicht-Änderungen als Änderungen angesehen. Eine Möglichkeit wäre separate Getter und Setter, was dann die Syntax aber weniger schön macht.

Ich hatte an so etwas gedacht (entschuldigt den schnellen Hack, ich wollte gerade keine ganze Matrixklasse schreiben, sondern nur zeigen, wie ich transponieren würde):

#include <iostream>
#include <vector>
#include <cassert>

template <class T> class Matrix;

template <class T> class TransposedMatrix
{
private:
  const Matrix<T> & matrix;
  TransposedMatrix(Matrix<T> const &matrix);
public:
  const T& operator()(size_t x, size_t y) const;
  friend class Matrix<T>;
};

template <class T> class Matrix
{
private:
  typedef typename std::vector<T>::size_type size_t;
  std::vector<T> data;
  size_t nrows, ncols;
public:
  Matrix( size_t nrows , size_t ncols): data(nrows*ncols), nrows(nrows), ncols(ncols)  {}
  Matrix( TransposedMatrix<T> const & other): nrows(other.matrix.ncols), ncols(other.matrix.nrows) 
  {
    data.reserve(ncols*nrows);
    for (size_t i = 0; i < nrows; ++i)
      for(size_t j = 0; j < ncols; ++j)
        data.push_back(other(i,j));
  }

  T& operator()(size_t x, size_t y)
  {
    assert(x < nrows and y < ncols);
    return data[x*ncols + y];
  }
  const T& operator()(size_t x, size_t y) const
  {
    assert(x < nrows and y < ncols);
    return data[x*ncols + y];
  }

  const TransposedMatrix<T> transpose() const
  {
    return *this;
  }
  friend class TransposedMatrix<T>;
};

template <class T> TransposedMatrix<T>::TransposedMatrix(Matrix<T> const &matrix): matrix(matrix) {}

template <class T> const T& TransposedMatrix<T>::operator()(size_t x, size_t y) const
{
  assert(y < matrix.nrows and x < matrix.ncols);
  return matrix.data[y*matrix.ncols + x];
}

using namespace std;

int main()
{
  Matrix<int> foo(5,3);
  for(int i = 0; i < 5; ++i)
    for (int j = 0; j < 3; ++ j)
      foo(i,j) = i*10 + j;

  for(int i = 0; i < 5; ++i)
    {
      for (int j = 0; j < 3; ++ j)
        cout << foo(i,j) << "\t";
      cout << endl;
    }
  cout << endl;

  for(int i = 0; i < 3; ++i)
    {
      for (int j = 0; j < 5; ++ j)
        cout << foo.transpose()(i,j) << "\t";
      cout << endl;
    }
  cout << endl;

  Matrix<int> transposed = foo.transpose();
  for(int i = 0; i < 3; ++i)
    {
      for (int j = 0; j < 5; ++ j)
        cout << transposed(i,j) << "\t";
      cout << endl;
    }
}

P.S.: So im Nachhinein gesehen ist das unnötig kompliziert, einfache Überladung von transpose hätte es auch getan. Naja, hat immerhin Spaß gemacht das zu programmieren.

[Kiigass]

CStoll schrieb:

Kiigass schrieb:

Ja die zusätzlich Dereferenzierung bzw. genauer: Die Syntaxänderung zum Aufruf der Daten ist unzulässig. Laut Aufgabenstellung muss die Syntax sein: "m[i][j]".
Allerdings gibt es dafür ja auch den überladenen operator[] der eigentlich das Objekt zurück geben sollte (also bei der Klasse Matrix soll er ein Objekt vom Typ Zeile zurück geben und bei der Klasse Zeile einen Integer). Vllt steckt auch da das Problem.

Warum verwendest du den dann nicht in deinen Funktionen? Das Teil sollte nicht die Aufrufe von außen ersetzen, sondern deine missglückten Aufrufe ma.mat[i][j] in den Operatoren.

hmm entweder verstehst du mich nicht oder ich dich nicht, reden wir an einander vorbei?
Das Problem tritt ja beim Überladen von operator+ bzw operator* auf. Ich hatte oben aber auf den operator[] angespielt. Also sicherlich könnte ich in meiner Funktion das so schreiben wie du es mir vorgescchlagen hast, aber dann würde das Problem außerhalb doch immer noch bestehen. Oder bin ich grad zu blöd?

[CStoll]

Nochmal langsam von vorne: Dein operator[] ist in Ordnung und macht das, was er soll - das heißt, es ist kein Problem für die Anwender der Klasse, per m[i][j] auf die Matrix-Elemente zuzugreifen. Aber in den arithmetischen Operatoren verwendest du nicht diesen Operator, sondern greifst direkt auf deine internen Daten zu - dort mußt du die Zeiger dann oft genug dereferenzieren, um an die richtige Stelle zu kommen.

[Kiigass]

CStoll schrieb:

Nochmal langsam von vorne: Dein operator[] ist in Ordnung und macht das, was er soll - das heißt, es ist kein Problem für die Anwender der Klasse, per m[i][j] auf die Matrix-Elemente zuzugreifen. Aber in den arithmetischen Operatoren verwendest du nicht diesen Operator, sondern greifst direkt auf deine internen Daten zu - dort mußt du die Zeiger dann oft genug dereferenzieren, um an die richtige Stelle zu kommen.

okay ich glaube ich habe das verstanden, aber dann stellt sich mir die Frage warum er in den "arithmetischen Operatoren" nicht den überladenen operator[] benutzt. Ist das immer so? und wenn ja warum?

ps: danke für deine Geduld!

pps: in der print()-Funktion kommt es auch zu einem Fehler, aber wenn der Zugriff wie du sagst, über m[i][j] möglich ist, müsste dann dort nicht einfach ein Integer stehen? Also warum kommt es zu einem Fehler?

[CStoll]

Kiigass schrieb:

CStoll schrieb:

Nochmal langsam von vorne: Dein operator[] ist in Ordnung und macht das, was er soll - das heißt, es ist kein Problem für die Anwender der Klasse, per m[i][j] auf die Matrix-Elemente zuzugreifen. Aber in den arithmetischen Operatoren verwendest du nicht diesen Operator, sondern greifst direkt auf deine internen Daten zu - dort mußt du die Zeiger dann oft genug dereferenzieren, um an die richtige Stelle zu kommen.

okay ich glaube ich habe das verstanden, aber dann stellt sich mir die Frage warum er in den "arithmetischen Operatoren" nicht den überladenen operator[] benutzt. Ist das immer so? und wenn ja warum?

Dein Index-Operator wird nur angewendet, wenn der linke Operand eine Matrix ist - wie in m[i][i] . Bei deinen Aufrufen m.mat[i][j] steht dort aber ein Zeiger - und der hat seinen eigenen (eingebauten) Index-Operator.

pps: in der print()-Funktion kommt es auch zu einem Fehler, aber wenn der Zugriff wie du sagst, über m[i][j] möglich ist, müsste dann dort nicht einfach ein Integer stehen? Also warum kommt es zu einem Fehler?

In der print-Funktion kannst du *this als Matrix-Objekt verwenden, mit dem du den Index-Operator nutzen kanns, das sieht dann so aus cout<<(*this)[i][/j]<<endl;

[Kiigass]

vielen dank für die kleine Nachhilfestunde, ich sah den Wald vor lauter Bäumen schon nicht mehr. Damit sind meine Probleme gelöst! Dank dir!

[inter2k3]

SeppJ schrieb:

Kiigass schrieb:

und wenn wir grad dabei sind: Kann mir jemand sagen ob es richtig von mir war, die Zeile 38 in matrix.cpp aus zu kommentieren? Wenn ich sie drin lasse meldet der Compiler: "doppeltes free"

Ja, das ist richtig, das auszukommentieren. Dein delete auf das mat ruft den Zeilendestruktor auf, welcher dann wieder ein delete für die einzelnen Werte aufruft.

Und das doppelte free sagt dir nicht dein Compiler, sondern deine Laufzeitumgebung. Das ist ein ganz großer Unterschied.

Nicht wirklich. mat ist vom Typ Zeile** und nicht Zeile*. Da wird kein Destruktor aufgerufen. Das auskommentieren ist also falsch und führt zum Speicherleck.