what is wrong with this???

okay, let's get to the point

Code:

#include <algorithm>
#include <iostream>
#include <vector>

using namespace std;

class Matrix {
public:
    Matrix () {
        row = column = 0;
    }

    Matrix (int r, int c) {
        if (r > 0 && c > 0) {
            row    = r;
            column = c;
            internalData.resize(row * column);
        }
    }

    bool isNull () const { return row == 0 && column == 0; }

    int getColumn () const { return column; }

    int getRow () const { return row; }

    double *getInternalData () {
        return &internalData[0];
    }

    Matrix &operator= (const Matrix &mat) {
        if (this == &mat) {
            return *this;
        }
        row    = mat.getRow();
        column = mat.getColumn();
        if (mat.isNull() == false) {
            double *tmp = mat.getInternalData();

            internalData.resize(row * column);
            copy(tmp, tmp + (row * column), internalData.begin());
        }

        return *this;
    }

protected:
    int            row;
    int            column;
    vector<double> internalData;
};

int main ()
{
    Matrix mat(3, 3);
    double *ptr = mat.getInternalData();

    ptr[0] = 1.0; ptr[1] = 2.0; ptr[2] = 3.0;
    ptr[3] = 0.0; ptr[4] = 4.0; ptr[5] = 5.0;
    ptr[6] = 1.0; ptr[7] = 0.0; ptr[8] = 6.0;

    return 0;
}

and I get this error messages:

In member function 'Matrix& Matrix::operator=(const Matrix &)':
error: passing 'const Matrix' as 'this' argument of 'double* Matrix::getInternalData()' discards qualifiers

what's wrong with my code?

Originally Posted by cph

what's wrong with my code?

It's obfuscated and illegible.

Variable definitions should be before anythign else. This way a reader doesn't have to scan your entire code looking for the type of the first variable they encounter.

it is customary to use this->Row for class specific variables, so they reader knows you mean teh class specific version and not some possible global version, particularly given that you havent yet defined row.

Itisbadformtodothingslikethisbecauseitmakesthecode verydifficulttoreadeveniftheinformationisallthere -

Code:

bool isNull () const { return row == 0 && column == 0; }

but perhaps the Visual Studio (YAY GO MS WOOOO) error will give you more insight

Originally Posted by Visual Studio Express

1>main.cpp
1>.\main.cpp(38) : error C2662: 'Matrix::getInternalData' : cannot convert 'this' pointer from 'const Matrix' to 'Matrix &'
1> Conversion loses qualifiers

Originally Posted by MSDN

Visual C++ Concepts: Building a C/C++ Program
Compiler Error C2662
Error Message
'function' : cannot convert 'this' pointer from 'type1' to 'type2'


The compiler could not convert the this pointer from type1to type2.

This error can be caused by invoking a non-const member function on a const object. Possible resolutions:

Remove the const from the object declaration.

Add const to the member function.

thank you very much, it's working now

Your constructor that takes two ints is rather bad. If either r or c is negative, then row and column are left uninitialised.

Your operator = is also broken. If mat.IsNull is true then you are doing nothing with the existing internalData.

Time to start using constructor initialisation lists too I think.

You shouldn't even need to implement operator= here: the values of the two integers and the vector would be copied correctly by the compiler-generated assignment operator.

The correct implementation would be:

Code:

Matrix &operator= (const Matrix &mat) 
{
    row = mat.row;
    column = mat.column;
    internalData = mat.internalData;
}

This is exactly what the default assignment operator would do.

You should add a return *this; to anon's example. This is why anon suggests that you should not even implement operator= here: unnecessary code means an unnecessary increase in the probability of bugs in the program.

In other words, the best implementation of operator = here, would be:

Code:

//

Variable definitions should be before anythign else. This way a reader doesn't have to scan your entire code looking for the type of the first variable they encounter.

it is customary to use this->Row for class specific variables, so they reader knows you mean teh class specific version and not some possible global version, particularly given that you havent yet defined row.

Opinions. In fact, consistently using this->name for member access from the class is highly uncustomary - you might do it, but in my experience you're in a small minority. As for defining members at the top of the class, I disagree. I start my classes with the interface - declarations of public member functions - and members are usually not part of that. But since I put my member definitions out of line, the code remains clean, and you see the definitions of the variables before reading the code that uses them.

wow! I really need to learn a lot more.
thank you