2

Why can't I use the function ColPeekHeight() as an l-value?

class View
{
    public:
        int ColPeekHeight(){ return _colPeekFaceUpHeight; }
        void ColPeekHeight( int i ) { _colPeekFaceUpHeight = i; }
    private:
        int _colPeekFaceUpHeight;
};

...

{
    if( v.ColPeekHeight() > 0.04*_heightTable )
        v.ColPeekHeight()-=peek;
}

The compiler complains at v.ColPeekHeight()-=peek. How can I make ColPeekHeight() an l-value?

BeeBand
  • 10,953
  • 19
  • 61
  • 83
  • Did you mean `ColPeakHeight`? – Emile Cormier Mar 21 '10 at 13:56
  • It's good the compiler complains about that statement - it has no effect. ColPeekHeight() returns a copy of the integer, so you're modifying the copy returned by the function! – AshleysBrain Mar 21 '10 at 14:53
  • @Emile, no that ColPeekHeight isn't a typo - I really do mean Peek. (it's the number of pixels visible of each card in a column, in a solitaire card game). – BeeBand Mar 21 '10 at 17:18

2 Answers2

9

Return the member variable by reference:

int& ColPeekHeight(){ return _colPeekFaceUpHeight; }

To make your class a good one, define a const version of the function:

const int& ColPeekHeight() const { return _colPeekFaceUpHeight; }

when I declare the function with the two consts

When you want to pass an object into a function that you don't expect it to modify your object. Take this example:

struct myclass
{
    int x;
    int& return_x() { return x; }
    const int& return_x() const { return x; }
};
void fun(const myclass& obj);

int main()
{
    myclass o;
    o.return_x() = 5;
    fun(o);
}
void fun(const myclass& obj)
{
    obj.return_x() = 5; // compile-error, a const object can't be modified
    std::cout << obj.return_x(); // OK, No one is trying to modify obj
}

If you pass your objects to functions, then you might not want to change them actually all the time. So, to guard your self against this kind of change, you declare const version of your member functions. It doesn't have to be that every member function has two versions! It depends on the function it self, is it modifying function by nature :)

The first const says that the returned value is constant. The second const says that the member function return_x doesn't change the object(read only).

Khaled Alshaya
  • 94,250
  • 39
  • 176
  • 234
  • AraK, when I declare the function with the two `const`s, I get "You cannot assign to a variable which is const"... – BeeBand Mar 21 '10 at 13:11
  • @BeeBand: Make sure `v` is not const first. – kennytm Mar 21 '10 at 13:13
  • @KennyTM, `v` is a non-const reference to a `View`. – BeeBand Mar 21 '10 at 13:16
  • 2
    @BeeBand, just in case this wasn't clear, AraK is suggesting you make BOTH those function, not just the const one. – Peter Alexander Mar 21 '10 at 13:20
  • 4
    While this will work, it's not generally considered good practice to expose the internals of a class in this way – jcoder Mar 21 '10 at 13:23
  • @BeeBand See Poita_ comment because thats what I mean, thanks Poita_ for clarifying my bad English :) – Khaled Alshaya Mar 21 '10 at 13:24
  • JB I totally agree with your point. See the first part of my answer for another question: http://stackoverflow.com/questions/1490225/c-copy-contructor-use-getters-or-access-member-vars-directly/1490248#1490248 – Khaled Alshaya Mar 21 '10 at 13:26
  • @JB, do you mean what I'm doing in general i.e. Getters and Setters? Yes, I understand that there's a big debate on this. – BeeBand Mar 21 '10 at 13:28
  • @BeeBand Oh yes, but that's just off-topic in this question because as you said not everyone agrees. – Khaled Alshaya Mar 21 '10 at 13:30
  • BTW, I am sure you would enjoy the writings of Dr. Stroustrup, take a look at some of them he writes a lot about programming styles in C++: http://www2.research.att.com/~bs/papers.html – Khaled Alshaya Mar 21 '10 at 13:33
  • Getters and setters are fine. Well, they are often a sign of poor OO design, but not bad in themselves. The problem is that if you return an reference to an internal data item then you can use that reference to modify the internal state of the object bypassing the methods (including getters and setters) – jcoder Mar 21 '10 at 13:51
  • 1
    Change the return type `const int&` to `int`. There is no reason to return a primitive by const reference, better return it by value. – fredoverflow Mar 21 '10 at 17:24
1

It can be rewritten like:

class View
{
    public:
        int  GetColPeekHeight() const  { return _colPeekFaceUpHeight; }
        void SetColPeekHeight( int i ) { _colPeekFaceUpHeight = i; }
    private:
        int _colPeekFaceUpHeight;
};

...

{
    cph = v.GetColPeekHeight();
    if ( cph > 0.04 * _heightTable )
        v.SetColPeekHeight( cph - peek );
}
xbit.net
  • 191
  • 3