Simple operator overload = in C-CodePudding

I have a problem in my code that does not display an apparent error. I created a Vector and Matrix class. The problem lies when I try to overload the = operator for the Matrix class. For example, if A and B are two matrices, I want the operation B = A to modify B which becomes A. This works for vectors but for matrices, I have a "Segmentation fault" error that I can't get not to solve: it is a problem of memory or prohibited modification but I cannot see.

// My code
#include <iostream>
#include <cmath>

using namespace std;

class Vecteur{
private:
    float *tab;
    int dim;

public:
    void affiche();
    Vecteur(int);
    Vecteur(float*, int);
    ~Vecteur();
    Vecteur(const Vecteur &);
    Vecteur operator=(const Vecteur &);
    Vecteur operator (const Vecteur &);
    Vecteur operator-(const Vecteur &);
    float & operator[](int);
    Vecteur subvec(int, int);
    int getDimension(); //utile pour la fonction dot
    friend Vecteur operator*(float, const Vecteur &);
    Vecteur(); //Constructeur vide pour la class Matrice
    friend class Matrice;
 };

void Vecteur::affiche(){
    cout << "Le tenseur est de dimension :"<<dim<<endl;
    cout << "Voici le contenu du tenseur :\n";
    for(int i=0;i<dim;i  ){
        cout <<tab[i]<<"\n";
    }
}

Vecteur::Vecteur(int taille){
    dim = taille;
    tab = new float[taille];
    for(int i=0;i<dim;i  ){
        tab[i] = 0;
    }
}

Vecteur::Vecteur(float *tableau, int taille){
    dim = taille;
    tab = new float[dim];
    for(int i=0;i<dim;i  ){
        tab[i] = tableau[i];
    }
}

Vecteur::~Vecteur(){
    delete tab;
}

Vecteur::Vecteur(const Vecteur &a){
    dim = a.dim;
    tab = new float[dim];
    for(int i=0;i<dim;i  ){
        tab[i] = a.tab[i];
    }
}

Vecteur Vecteur::operator=(const Vecteur &a){
    if(this != &a){
        this->~Vecteur();
        dim = a.dim;
        tab = new float[dim];
        for(int i=0;i<dim;i  ){
            tab[i] = a.tab[i];
        }
    }
    return *this;
}

Vecteur Vecteur::operator (const Vecteur &a){
    if(dim == a.dim){
        float tab_b[dim];
        for(int i=0;i<dim;i  ){
            tab_b[i] = tab[i]   a.tab[i];
        }
        Vecteur b(tab_b, dim);
        return b;
    } else {
        cout <<"Les vecteurs ne sont pas de meme taille.\n";
        float tab_b[1] = {0};
        Vecteur b(tab_b, 1);
        return b;
    }
}     

Vecteur Vecteur::operator-(const Vecteur &a){
    if(dim == a.dim){
        float tab_b[dim];
        for(int i=0;i<dim;i  ){
            tab_b[i] = tab[i] - a.tab[i];
        }
        Vecteur b(tab_b, dim);
        return b;
    } else {
        cout <<"Les vecteurs ne sont pas de meme taille.\n";
        float tab_b[1] = {0};
        Vecteur b(tab_b, 1);
        return b;
    }
}    

float & Vecteur::operator[](int i){
    return tab[i];
}

Vecteur Vecteur::subvec(int i, int j){
    float sous_tab[j-i 1];
    for(int k=i;k<j 1;k  ){
        sous_tab[k-i]=tab[k];
    }
    Vecteur c(sous_tab, j-i 1);
    return c;
}

int Vecteur::getDimension(){
    return dim;
}

Vecteur operator*(float k, const Vecteur &d){
    int dimension = d.dim;
    float tab_b[dimension];
    for(int i=0;i<dimension;i  ){
        tab_b[i] = k * d.tab[i];
    }
    Vecteur b(tab_b, dimension);
    return b;
}

Vecteur::Vecteur(){ }


class Matrice{
    private:
        Vecteur *mat;
        int dims[2];

    public:
        void affiche();
        Matrice();
        Matrice(int, int);
        Matrice(Vecteur);
        Matrice(Vecteur *, int);
        ~Matrice();
        Matrice(const Matrice &M);
        Matrice & operator=(const Matrice &);
        Vecteur & operator[](int);
        Matrice operator (const Matrice &);
        Matrice operator-(const Matrice &);
        Matrice operator*(const Matrice &);
};

void Matrice::affiche(){
    cout << "La matrice est de dimension "<<dims[0]<<" x "<<dims[1]<<endl;
    cout << "Les coefficients de la matrice sont : \n";
    for(int i=0;i<dims[0];i  ){
        for(int j=0;j<dims[1];j  ){
            cout <<mat[j][i]<<" ";
            if(j==dims[1]-1){
                cout <<"\n";
            }
        }
    }
}

Matrice::Matrice(int nbLignes, int nbColonnes){
    dims[0] = nbLignes;
    dims[1] = nbColonnes;
    mat = new Vecteur[nbColonnes];
    for(int i=0;i<nbColonnes;i  ){
        mat[i] = Vecteur(nbLignes);
    }
}

Matrice::Matrice(Vecteur a){
    int taille = a.getDimension();
    dims[0] = taille;
    dims[1] = taille;
    
    mat = new Vecteur[taille];
    for(int i=0;i<taille;i  ){
        mat[i] = Vecteur(taille);
    }
    
    //Puis on modifie les termes diagonaux
    for(int i=0;i<taille;i  ){
        mat[i][i] = a[i];
    }
}

Matrice::Matrice(Vecteur *M, int taille){
    dims[0] = M[0].getDimension();
    dims[1] = taille;
    mat = new Vecteur[taille];
    for(int i=0;i<dims[1];i  ){
        mat[i] = M[i];
    }
}

Matrice::~Matrice(){
    delete mat;
}

//Constructeur de recopie
Matrice::Matrice(const Matrice &M){
    dims[0] = M.dims[0];
    dims[1] = M.dims[1];
    mat = new Vecteur[dims[1]];
    for(int i=0;i<dims[1];i  ){
        mat[i] = M.mat[i];
    }
}

Matrice & Matrice::operator=(const Matrice &M){
   if(this != &M)
   {
        this->~Matrice();
        dims[0] = M.dims[0];
        dims[1] = M.dims[1];
        mat = new Vecteur[dims[1]];
        for(int i=0;i<dims[1];i  ){
            mat[i] = Vecteur(dims[0]);
        }
        for(int i=0;i<dims[0];i  ){
            for(int j=0;j<dims[1];j  ){
                mat[j][i] = M.mat[j][i];
            }        
        }
    }
    return *this;
}

Vecteur & Matrice::operator[](int i){
    return mat[i];
}

Matrice::Matrice(){
    mat = nullptr;
}

int main() {
    float tab_a[] = {1,2,3,4};
    Vecteur a(tab_a, 4);
    Matrice A(a);
    float tab_b[] = {5,6,7,8};
    Vecteur b(tab_b, 4);
    Matrice B(b);
    B.affiche();
    B = A;
    B.affiche(); //Show nothing
}

Thanks

CodePudding user response：

This smells,

    this->~Matrice();
    dims[0] = M.dims[0];
    dims[1] = M.dims[1];

You deleted the object and are again trying to assign the deleted object.You cannot assign to a deleted object.

It should be noted that,

dims[0]

is equivalent to

this->dims[0];

You should just remove the line that is calling the destructor and also change the call to delete to delete[] in its destructor.

CodePudding user response：

The main problem in your code is that you ignore the existence of <vector>, which relieves you of a lot of headaches.

First C Rule of Thumb: If you are using pointers, C arrays, and / or new, you are likely doing it wrong (unless you are in the business of expanding the standard library, or interfacing legacy C code).

Use what language and standard library provide. Use it all.

I reworked the code below in several ways:

Removing class Vecteur altogether, replacing it with a standard vector and a couple of global operator functions. All the class Vecteur constructors and operations could be trivially replaced.
Removing all manual memory handling in favor of std::vector, significantly shortening the code and reducing the chances for error.
affiche() became a standard ostream & operator<<().
References instead of pointers, always.
Range-fors, then iterators, then index for loops.
indices as unsigned, not int.

I've probably forgotten a few things, and at some point I got a bit bored and stopped improving the code further, but I hope what I have done already helps you with your C style.

Things I would probably still change:

unsigned -> std::size_t
identifiers lowercase, and English
comments and default output texts in English (the lingua franca of programming)
put a namespace around it all
separated vector and matrix, header and implementation file
showcased copy-and-swap idiom for the assignment operator
mat -> mat_ (a common idiom to indicate class variables)
private: to the bottom, public: to the top (we are interested in interface, not implementation)
...

You might want to turn vecteur into a template type, so you can have int, float, and double matrices... but then again, you could turn to Boost.org and check out uBlas, or QVM, which are ready-made matrix support libraries. Other third-party work exists as well.

#include <iostream>
#include <vector>

typedef std::vector< float > vecteur;

vecteur operator ( vecteur const & a, vecteur const & b)
{
    if ( a.size() != b.size() )
    {
        std::cout <<"Les vecteurs ne sont pas de meme taille.\n";
        return vecteur( 1 );
    }

    vecteur resultat( a );
    auto b_it = b.begin();

    for ( float & f : resultat )
    {
        f  = *b_it;
          b_it;
    }

    return resultat;
}

vecteur operator-( vecteur const & a, vecteur const & b)
{
    if ( a.size() != b.size() )
    {
        std::cout <<"Les vecteurs ne sont pas de meme taille.\n";
        return vecteur( 1 );
    }

    vecteur resultat( a );
    auto b_it = b.begin();

    for ( float & f : resultat )
    {
        f -= *b_it;
          b_it;
    }

    return resultat;
}

vecteur operator*( float k, vecteur const & d)
{
    vecteur resultat( d );

    for ( float & f : resultat )
    {
        f *= k;
    }

    return resultat;
}

class Matrice{
    private:
        std::vector< vecteur > mat;

    public:
        friend std::ostream & operator<<( std::ostream &, Matrice const & );
        Matrice();
        Matrice( unsigned, unsigned );
        Matrice( vecteur const & );
        Matrice( std::vector< vecteur > const &, unsigned );
        Matrice( Matrice const & M );
        Matrice & operator=( Matrice const & );
        vecteur & operator[]( unsigned );
        vecteur const & operator[]( unsigned ) const;
        friend Matrice operator ( Matrice const &, Matrice const & );
        friend Matrice operator-( Matrice const &, Matrice const & );
        friend Matrice operator*( Matrice const &, Matrice const & );
};

std::ostream & operator<<( std::ostream & out, Matrice const & m )
{
    unsigned y = m.mat.size();
    unsigned x = ( m.mat.empty() ) ? 0 : m.mat[0].size();

    out << "La matrice est de dimension " << x << " x " << y << std::endl;

    for ( vecteur const & v : m.mat )
    {
        for ( float const & f : v )
        {
            out << f << " ";
        }

        out << "\n";
    }

    return out;
}

Matrice::Matrice( unsigned nbLignes, unsigned nbColonnes ) : mat( nbLignes, vecteur( nbColonnew ) )
{}

Matrice::Matrice( vecteur const & a ) : mat( a.size(), vecteur( a.size() ) )
{
    //Puis on modifie les termes diagonaux
    for ( unsigned i = 0; i < a.size();   i )
    {
        mat[i][i] = a[i];
    }
}

//Constructeur de recopie
Matrice::Matrice( Matrice const & M ) : mat( M.mat )
{}

Matrice & Matrice::operator=( Matrice const & M )
{
    if ( this != &M )
    {
        mat = M.mat;
    }

    return *this;
}

vecteur & Matrice::operator[]( unsigned i )
{
    return mat[i];
}

vecteur const & Matrice::operator[]( unsigned i ) const
{
    return mat[i];
}

Matrice::Matrice()
{}

int main() {
    vecteur a{ 1, 2, 3, 4 };
    Matrice A( a );
    vecteur b{ 5, 6, 7, 8 };
    Matrice B( b );
    std::cout << B << std::endl;
    B = A;
    std::cout << B << std::endl;
    vecteur c = 1.5 * a;
    Matrice C(c);
    std::cout << C << std::endl;
}