Thread: what is wrong with this???

  1. #1
    Registered User cph's Avatar
    Join Date
    Sep 2008
    Location
    Indonesia
    Posts
    86

    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?

  2. #2
    Malum in se abachler's Avatar
    Join Date
    Apr 2007
    Posts
    3,195
    Quote Originally Posted by cph View Post
    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

    Quote 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
    Quote 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.
    Last edited by abachler; 03-20-2009 at 08:25 PM.

  3. #3
    Registered User cph's Avatar
    Join Date
    Sep 2008
    Location
    Indonesia
    Posts
    86
    thank you very much, it's working now

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    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.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  5. #5
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    In other words, the best implementation of operator = here, would be:
    Code:
    //
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  8. #8
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    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.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  9. #9
    Registered User cph's Avatar
    Join Date
    Sep 2008
    Location
    Indonesia
    Posts
    86

    Smile

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

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 9
    Last Post: 07-15-2004, 03:30 PM
  2. Debugging-Looking in the wrong places
    By JaWiB in forum A Brief History of Cprogramming.com
    Replies: 1
    Last Post: 11-03-2003, 10:50 PM
  3. Confused: What is wrong with void??
    By Machewy in forum C++ Programming
    Replies: 19
    Last Post: 04-15-2003, 12:40 PM
  4. God
    By datainjector in forum A Brief History of Cprogramming.com
    Replies: 746
    Last Post: 12-22-2002, 12:01 PM
  5. Whats wrong?
    By Unregistered in forum C Programming
    Replies: 6
    Last Post: 07-14-2002, 01:04 PM