CODEWITHSUNDEEP
c++classvariables
Brennan Hoeting
Brennan Hoeting

Reputation: 1213

C++: Returning a class variable

When I try using a method to return a private variable, it seems the value changes from since the object was constructed. Here is my code and output.

main.cpp

#include <iostream>
#include "coordinate.h"

using namespace std;

int main()
{
    Coordinate c(1, 1);
    cout << c.getX() << endl;

}

coordinate.cpp

#include "coordinate.h"
#include <iostream>

using namespace std;

Coordinate::Coordinate(int x, int y)
{
    x = x;
    y = y;

    cout << x << endl;

}

coordinate.h

#ifndef COORDINATE_H
#define COORDINATE_H

class Coordinate
{
    private:
        int x;
        int y;

    public:
        Coordinate(int x, int y);

        int getX() { return x; }
        int getY() { return y; }
};

#endif

Output

Upvotes: 3

Views: 4384

Answers (8)

kfsone
kfsone

Reputation: 24249

As others have pointed out

Coordinate::Coordinate(int x, int y)
{
    x = x;
    y = y;

This is a condition called "shadow variables". The values "x" and "y" in the function prototype shadow the member variables, and thus "x = x" assigns the value of parameter x to the parameter x.

Compilers like GCC and Clang will warn you about this. MSVC doesn't because Microsoft's API is a bit messy and they use a hell of a lot of the symbol table :)

A widely used way to avoid this is prefixes. "m_" for "member", "g_" for "global", "s_" for "static".

class Coordinate
{
    // by saying 'class' instead of 'struct',
    // you declared the initial state to be "private".
    int m_x;
    int m_y;
    ...


Coordinate::Coordinate(int x, int y)
    : m_x(x)
    , m_y(y)
{}

Some folks take this a step further and use a prefix - or suffix - for parameter names; I picked up adding "_" from several Open Source projects I worked on.

Coordinate::Coordinate(int x_, int y_)
    : m_x(x_)
    , m_y(y_)
{
    int x = m_x;  // assign from member value
    int y = y_;   // assign from parameter
    std::cout << x << ", " << y << std::endl;
}

Upvotes: 0

hack4m
hack4m

Reputation: 295

With these code:

x = x;
y = y

you are assign x and y to themselves.

Coordinate::Coordinate(int xVal, int yVal)
{
    x = xVal;
    y = yVal;

    cout << x << endl;
}

is okay.

the best way is:

Coordinate::Coordinate(int xVal, int yVal):x(xVal),y(yVal)
{
    cout << x << endl;
}

Upvotes: 0

user2357112
user2357112

Reputation: 280207

Your constructor is assigning to its arguments instead of the object's private fields. Use an initialization list, or explicitly qualify the assignment targets with this, or pick different argument names:

Coordinate::Coordinate(int x, int y) : x(x), y(y) {
    cout << x << endl;
}

or

Coordinate::Coordinate(int x, int y) {
    this->x = x;
    this->y = y;
    cout << x << endl;
}

or

Coordinate::Coordinate(int xVal, int yVal) {
    x = xVal;
    y = yVal;
    cout << x << endl;
}

Upvotes: 9

Saby
Saby

Reputation: 718

uselease replace the variable names as : 

class Coordinate
{
    private:
        int a;
        int b;

    public:
        Coordinate(int x, int y)
       {
         a = x;
         b = y;

         cout << x << endl;
       }

        int getX() { return a; }
        int getY() { return b; }
};

Actually the compiler is getting confused which value of x to use, plz use some other variables in private section. Otherwise you can use this to resolve this also as:

 Coordinate(int x, int y)
       {
         this->x = x;
         this->y = y;

         cout << x << endl;
       }

Upvotes: 0

Mark Garcia
Mark Garcia

Reputation: 17708

The problem is with these assignments:

x = x;
y = y;

You are actually assigning the constructor parameters x and y to themselves, and not from the parameters to the object's x and y members.

Also, this line

cout << x << endl;

prints the constructor parameter x and not the object's x member.

You are hiding members x and y by using the same names as the constructor parameters' names. Referencing the names x and y without qualifying them would refer to the parameters and not the object's members.

You can solve this problem by doing something like this. In that, I prefixed the member variables with m_. You can also do some other similar techniques.

Upvotes: 0

nommyravian
nommyravian

Reputation: 1336

Did you try assigning the values to private member variables using this pointer like this?

Coordinate::Coordinate(int x, int y)
{
    this->x = x;
    this->y = y;

    cout << x << endl;
}

or what you can do is to change the parameter names in constructor to avoid using this pointer

Coordinate::Coordinate(int a, int b)
{
    x = a;
    y = b;

    cout << x << endl;
}

Upvotes: 1

Mankarse
Mankarse

Reputation: 40613

Within the constructor, x refers to the argument, rather than the member variable, so x = x is an assignment of the argument to itself. The member variables remain uninitialised.

You can avoid this problem by using a member-initialiser-list or by explicitly referring to the member variable through this->x.

Coordinate::Coordinate(int x, int y) : x(x), y(y)
{
    cout << this->x << endl;
}

Upvotes: 2

someone_ smiley
someone_ smiley

Reputation: 1066

Try :

Coordinate::Coordinate(int x, int y)
{
     this->x = x;
     this->y = y;

     cout << this->x << endl;

}

First time you get value of x = 1 because its value of argument 'x' that got print. but second time you got it wrong because, member variable x never get any value assigned.

Upvotes: 0

Related Questions