Probleme mit gmp/gmpxx

leanderlaepple

Hi,

ich bekomme mit gmp bei folgendem Code einen "Speicherzugriffsfehler"

Datei: BigFraction.cpp

#include "BigFraction.h"
#include<iostream>
#include "gmp/gmpxx.h"

#define BigInt mpz_class

using namespace std;

BigFraction::BigFraction(int64_t divident, int64_t divisor) {
	this -> divident = divident;
	this -> divisor  = divisor;
}

void BigFraction::echoNumber() {
	cout << "(" << this -> divident << " / " << this -> divisor << ")";
}

BigFraction BigFraction::operator+(BigFraction summand) {
	BigInt divident;
	BigInt divisor;

	divident = this -> divident*summand.divisor + this -> divisor*summand.divident;
	divisor = this -> divisor * summand.divisor;
}

int main() {
	BigFraction a(1,2), b(1,4);
	BigFraction c(0,1);
	c = a+b;
	c.echoNumber();
	cout << "Haaaaallo" <<endl;
}

Datei: BigFraction.h

#ifndef _BIGFRACTION_H_
#define _BIGFRACTION_H_

#include "gmp/gmpxx.h"
#include<iostream>
#include<inttypes.h>

#define BigInt mpz_class
class BigFraction {
	private:
		BigInt divident, divisor;
	public:
		void reduce();
		BigFraction(BigInt divident, BigInt divisor);
		BigFraction(int64_t divident, int64_t divisor);
		BigFraction operator+(BigFraction summand);
		BigFraction operator-(BigFraction subtrahend);
		BigFraction operator*(BigFraction multiplicator);
		BigFraction operator/(BigFraction divisor);
		void echoNumber();
};

#endif

kompiliert mit gcc unter Linux:

g++ -o bftest BigFraction.cpp -lgmp -lgmpxx

SeppJ

Dein Operator+ gibt nix zurück.

leanderlaepple schrieb:

kompiliert mit gcc unter Linux:
g++ -o bftest BigFraction.cpp -lgmp -lgmpxx

g++ -o bftest BigFraction.cpp -lgmp -lgmpxx -Wall -Wextra

So fallen solche Fehler sofort auf.

P.S.: Guck dir mal an, was eine Initialisierungsliste ist!

leanderlaepple

Geil, danke!!!

Ich Idiot habe den Fehler nicht gesehen!

Also wie gesagt vielen Dank,

Herzliche Grüße
leanderlaepple

leanderlaepple

muss ich den Thread irgendwo als gelöst markieren?

SeppJ

leanderlaepple schrieb:

muss ich den Thread irgendwo als gelöst markieren?

Nein. Lass einfach laufen. Wenn jemand noch etwas zu sagen hat, wird er es tun, ansonsten wird der Thread einfach im Laufe der Zeit verdrängt.

krümelkacker

Zu sagen hätte ich noch, dass es ungeschickt ist, diese binären Operatoren als nicht-statische Elementfunktionen zu implementieren -- mal ganz davon abgesehen, dass bei Dir die const-Qualifizierung fehlt und operator+ das Objekt, auf dem es aufgerufen wurde, verändert.

Header passt auch nicht ganz zur cpp-Datei (uint64_t versus BigInt).

Und das #define BigInt ist auch Käse. Dafür gibt's typedef bzw using . Am besten machst du das in einem eigenen Namensraum.

Namen, die mit einem Unterstrich anfangen und mit einem Großbuchstaben weitergehen, sind reserviert. Du hältst dich mit _BIGFRACTION_H_ nicht daran.

Also alles in allem: Viele unschöne Anfänger-Eigenschaften.

Vorschlag:

#ifndef DEIN_BIGFRACTION_H_
#define DEIN_BIGFRACTION_H_

#include "gmp/gmpxx.h"

namespace dein {

typedef mpz_class BigInt;

class BigRational
{
public:
    BigRational(int i);
    BigRational(long i);
    BigRational(BigInt const& i);
    BigRational(BigInt const& num, BigInt const& denom);

    BigInt const& numerator() const {return numerator_;}
    BigInt const& denominator() const {return denominator_;}

private:
    BigInt numerator_, denominator_; // Invariante: 
                                     // gcd(num,denom)==1 && denom>=1
};

// operator+ als freie Funktion in demselben Namensraum definieren,
// sie wird dann automatisch per ADL gefunden.
BigRational operator+(BigRational const& lhs, BigRational const& rhs);

} // namespace du

#endif

Warum ist das so besser? Weil du dann z.B. schon ganz automatisch das hier schreiben kannst:

int main()
{
    BigRational r (2,3);
    BigRational x = 5 + r; // geilomat!
}

Der richtige Operator+ wird hier per ADL gefunden und die linke Seite wird automatisch dank BigRational::BigRational(int) zu einem BigRational konvertiert.

leanderlaepple

krümelkacker schrieb:

Zu sagen hätte ich noch, dass es ungeschickt ist, diese binären Operatoren als nicht-statische Elementfunktionen zu implementieren -- mal ganz davon abgesehen, dass bei Dir die const-Qualifizierung fehlt und operator+ das Objekt, auf dem es aufgerufen wurde, verändert.

Header passt auch nicht ganz zur cpp-Datei (uint64_t versus BigInt).

Und das #define BigInt ist auch Käse. Dafür gibt's typedef bzw using . Am besten machst du das in einem eigenen Namensraum.

Namen, die mit einem Unterstrich anfangen und mit einem Großbuchstaben weitergehen, sind reserviert. Du hältst dich mit _BIGFRACTION_H_ nicht daran.

Also alles in allem: Viele unschöne Anfänger-Eigenschaften.

Ich entschuldige mich aufrichtig dafür und kann jeden verstehen, dem das Augenschmerzen bereitet, ich gebe zu: ich bin ein Anfänger in C++ und habe auch in anderen Sprachen lange nix mehr gemacht. Desweiteren bedanke ich mich für alle Anregungen, die ich von euch kriege.

krümelkacker schrieb:

Vorschlag:

#ifndef DEIN_BIGFRACTION_H_
#define DEIN_BIGFRACTION_H_

#include "gmp/gmpxx.h"

namespace dein {

typedef mpz_class BigInt;

class BigRational
{
public:
    BigRational(int i);
    BigRational(long i);
    BigRational(BigInt const& i);
    BigRational(BigInt const& num, BigInt const& denom);

    BigInt const& numerator() const {return numerator_;}
    BigInt const& denominator() const {return denominator_;}

private:
    BigInt numerator_, denominator_; // Invariante: 
                                     // gcd(num,denom)==1 && denom>=1
};

// operator+ als freie Funktion in demselben Namensraum definieren,
// sie wird dann automatisch per ADL gefunden.
BigRational operator+(BigRational const& lhs, BigRational const& rhs);

} // namespace du

#endif

Warum ist das so besser? Weil du dann z.B. schon ganz automatisch das hier schreiben kannst:

int main()
{
    BigRational r (2,3);
    BigRational x = 5 + r; // geilomat!
}

Der richtige Operator+ wird hier per ADL gefunden und die linke Seite wird automatisch dank BigRational::BigRational(int) zu einem BigRational konvertiert.

ok, das ist echt geil!

Aber das mit dem namespace habe ich net verstanden - warum soll ich namespace schreiben?
(und ganz nebenbei, wie sollte ich so einen namespace benennen?)

krümelkacker

Mit 'nem eigenen Namensraum kannst du Namenskollisionen vermeiden. Darin kannst du dich austoben, wie du willst, und wirst auch nicht von anderen gestört, die irgendwelche Namen in den globalen Namensraum reinpacken, beispielsweise weil du oder jemand anders deine BigRational-Klasse irgendwo anders verwenden will. Es beeinflusst auch die Namensauflösung. Beispiel:

void foo();

namespace dir {

void foo();

void bar()
{
  foo(); // dein dir::foo und nicht das "globale" foo
}

} // namespace dir

...weil bei der Namensauflösung eine Suche von "innen nach außen" stattfindet, die dann abbricht, wenn man schon etwas gefunden hat.

Es ist u.a. auch dan praktisch, wenn du Funktionstemplates benutzt. Definierst du ein eigenes Template im globalen Namensraum wie

template<class T>
void blah(T);

dann wird es ggf für alles mögliche instantiiert, weil die Namensauflösung blah irgendwann findet. Beschränkst du dich damit auf deinen eigenen Namensraum, wird das Tempate nicht mehr unbeabsichtigt für Typen verwendet, die ganz wo anders definiert wurden.

Habe jetzt keine anderen besonders tollen Beispiele. Aber ich hatte schon eine Situation, wo ich ein Programm nicht zum Laufen bekommen habe, weil zwei verschiedene "Module" sich im "globalen Namensraum" gestört haben. Habe meinen Kram dann einfach in einen eigenen Namensraum geschoben und gut war.

Und von ADL sollte man auch mal gehört haben. Schai mal hier rein:
http://www.gotw.ca/publications/mill08.htm

leanderlaepple

Hi again,

sollte man Operatorenüberladung immer in nicht-objektorien Funktionen realisieren? Also auch bei Operatoren wie

+=

etc.?

Danke für jede Antwort,
leanderlaepple

SeppJ

Nein. guckst du hier für einen Leitfaden mit Erklärungen:
http://http://www.c-plusplus.net/forum/p1647670