is this C++ getter-like syntax wrong ?

Question

Say i have the following class :

class Abc {

    int id;

public:

    int getID() {   return id; }


    int setID(int id) { this->id = id; }
};

Is there any logical error in this ? I seem to be getting unexpected results (read : wrong values of id). I know this is not the way to write a getter .. but still there shudn't be any error in this code ?

Here is the class declaration :

class ClientConn {


  static int num;
  short pos;
  sockaddr_in tcpAddress;
  sockaddr_in udpAddress;
  int connFD;

 public:
  ClientConn();

  int getConnFD();

  void setConnFD(int connFD);

  void setPos(short pos);

  short const& getPos();

  void setUdpAddress(short port);

  void setTcpAddress(sockaddr_in address);

  void setUdpAddress(sockaddr_in address);

  void setTcpAddress(short port, char* serverIP);

  void setUdpAddress(short port, char * serverIP);

  sockaddr_in const& getTcpAddress() const;    

  sockaddr_in const& getUdpAddress() const;



};

the two functions had been defined as follows :

int ClientConn :: getConnFD() {
  return connFD;
}

void ClientConn :: setConnFD(int connFD) {
  this->connFD = connFD;    
}

I had set the value of connFD to 7 using the setter, and then when i was using the getter, i was getting the value 65534.

(Should i answer my question or keep editing my post ? im new)

Answer 1

You have to initialize the int id with 0

Answer 2

that won't/shouldn't compile. Your setter should have a 'void' return type.

Answer 3

A few notes:

• int getID() should be a const method.
• Why does setID() have an int return type? It doesn't return a value. How does this even compile?
• Are you sure the unexpected results are because of the getter/setter? Do you have a short test program to demonstrate the problem?

EDIT: Now that you posted your code, I would assume that something is stomping your variables. What compiler are you using? A memory breakpoint would be the fastest way to tell you what's going on. Assuming that's not an option, sprinkle your code with debug output that shows the current value of the variable and do a divide and conquer until you find out where it gets stomped.

Also, the new code you posted still doesn't demonstrate any actual usage. A simple test program would help.

Answer 4

 int setID(int id) { this->id = id; }

should be replaced by

void setID(int id) { this->id = id; }

And you should declare a constructor to initialize the value of id.

Answer 5

Make sure you initialize id in the constructor. That said, don't blindly create accessors. When you add code always consciously know there's a good reason for it.

Answer 6

int getID() {   return id; }

This is fine, but it should be const:

int getID() const {   return id; }


int setID(int id) { this->id = id; }

This should not return a value:

void setID(int id) { this->id = id; }

Answer 7

Your code looks (and works) correctly for me:

#include <iostream>

class Abc {

    int id;

public:

    int getID() {   return id; }


    int setID(int id) { this->id = id; }
};

int main ()
{
    std::cout << "Hello World\n";
    Abc abc;
    abc.setID(5);
    std::cout << "Result: " << abc.getID() << "\n";
}

The results are as expected:

me@here:~/tmp$ ./a.out 
Hello World
Result: 5

Answer 8

I wonder if you are getting unexpected results because you have not initialized id. When you instantiate your class, the value of id is uninitialized. C++ does not set to zero by default. Therefore, if you were to call getID() before ever calling setID(), you would get unpredictable results.

Here’s a revised version of your class that properly initializes id, marks the getter as const and removes the unnecessary return value from the setter. I also used the common convention of adding an underscore (_) to private member variables. This is just a matter of personal preference.

class Abc {
    int id_;

public:
    // default constructor that initializes 'id_' to zero
    Abc() : id_(0) {}

    int getID() const { return id_; }
    void setID(int id) { id_ = id; }
};

Answer 9

My crystal ball (and EboMike's edit) sais:

You overwrite connFD in your void setUdpAddress(short port, char * serverIP);. You should be using sockaddr instead of sockaddr_in. Your lucky numbers are 3, 27 and 0x4f.

HTH.

Answer 10

Inside your:

int setID(int id) { this->id = id; }

Why do you assume "id" is not also the member? You cannot. This is just as valid:

int setID(int id) { id = id; } // problem get's obvious now.

I bet you are assigning your (uninitialized) variable to the value it already has.

try this:

void setID(int newId) { this->id = newId; }

Note: I usually recommend NOT to place "this" everywhere. It should be obvious when you call a member (function or data) or a local variable (assuming you have no globals ;)