1

Here's my problem.

I have a class that holds an array of objects (in a std::vector) These object can be modified from client code, so I made a private getter that returns a pointer to the object I need to modify. The public methods use this getter to modify the objects in the array.

The private getter is also used in other member functions that return some specific values of the objects in the array to client code. I would like these functions to be and return const, but I can not do that because I use the non-const getter earlier mentioned.

I know I could make one other const getter, but that would just duplicate code.

How can implement this correctly?

Code example:

class Object;

class Inventory
{
    Object* GetObject(int id);
    void AddObjectProp(int id, int amount) {
        Object* x = GetObject id);
        x->prop += amount;
    }

    //using const here is not posible because GetObject is not const
    int GetObjectProp(int id) {
        Object* x = GetObject id);
        return x->prop;
    }    

}

Thank you.

Cristi Popa
  • 11
  • 1
  • 2
  • 2
    Isn't http://stackoverflow.com/questions/123758/how-do-i-remove-code-duplication-between-similar-const-and-non-const-member-func?rq=1 addressing the same question? – anumi Jan 24 '13 at 16:18
  • Const correctness always seems to demand you duplicate your getters (ones that return pointers or references at least). Since duplication is never right, something has to go: either those pesky getters, or const correctness... – n. 'pronouns' m. Jan 24 '13 at 16:24

3 Answers3

3

I know you say you don't want to do that, but the cleanest solution is to use two getters:

class Inventory
{
    Object* GetObject(int id);
    const Object* GetObject(int id) const;

    void AddObjectProp(int id, int amount) {
        Object* x = GetObject(id);
    }

    int GetObjectProp(int id) const {
        const Object* x = GetObject(id);
    }    
};

As far as duplicating the GetObject() implementation, you could either

  • factor out most of the code; or
  • implement one getter in terms of the other.
NPE
  • 438,426
  • 93
  • 887
  • 970
2

I believe you could make a

const Object* GetObject(int id) const;

then you could:

int GetObjectProp(int id) const {
    const Object* x = GetObject(id);
    return x->prop;
}    

Or:

int GetObjectProp(int id) const {
    return GetObject(id)->prop;
}    

(Also fixed the missing parenthesis before "id" in GetObject())

Mats Petersson
  • 119,687
  • 13
  • 121
  • 204
  • +1. And since the definition of `GetObject` is not shown, there is no way to tell how easy or difficult it will be to define a const version. It might be impossible -- if `GetObject` is a non-const function for some convincing reason (despite being called "get"), then maybe `GetObjectProp` really should not call it in the first place. – Steve Jessop Jan 24 '13 at 16:49
  • Oh, sure, there may be various reasons why this doesn't actually work. And there may also be workarounds for that, should for example GetObject call some "creat new object" when one doesn't exist, or do some other "modifying the class" - although that would possibly be a candidate for using "mutable" - not sure if someone will immediately strike me down for saying mutable... ;) – Mats Petersson Jan 24 '13 at 16:54
  • heh. I suspect that mutable would be dodgy in the case where `Inventory` provides access to a list of all the ids it has objects for (because then the function is not logically const, it changes the list), might be OK in the case where Inventory is acting like a sort of transparent cache. One should also consider that non-thread-safe `const` functions are asking for trouble. Like you say there's plenty of opportunity to mess things up :-) – Steve Jessop Jan 24 '13 at 17:02
  • I can not make GetObject const because I also use it like in AddObjectProp example, where I need it to be mutable. So it looks like i will need to duplicate a little bit of code. – Cristi Popa Jan 24 '13 at 17:44
  • Then you can't have const-correctness. You basically have to choose: Do you make it const-correct, or do you make two functions - you can't have it both ways (although with a bit of const-casting, you can usually make one function out of the other!) – Mats Petersson Jan 24 '13 at 17:46
  • 1
    You could have a private static member function defined as `template U *getobjectimpl(T *self, int id) { ... do whatever you need to do ... }`. That way you don't duplicate the actual getter code, but it's instantiated twice. Both instantiations get checked for const-safety by the compiler, and there's no const-cast to subvert that. You end up with some boilerplate, though: `const Object *getObject(int id) const { return getobjectimpl(this, id); }` and `Object *getObject(int id) { return getobjectimpl(this, id); }`. – Steve Jessop Jan 25 '13 at 13:31
  • ... so it's only worth it if the getter code is complex enough that you really want to avoid just duplicating it in the const and non-const overloads of `getObject`. If the getter code is `return this->obj_ptrs[id];` then I would not normally worry about duplicating it :-) – Steve Jessop Jan 25 '13 at 13:33
0

I dont think having a double getter could be seen as code-duplication in here. Both getters would have their own use case.

One getter is designed to be used by clients which need a const reference.

The other getter (that I'd call accessor, to make it esplicit in the code that there we are looking for a non const reference) is ment to be used by any client who's potentially willing to modify the object.

This would be my approach :

class Inventory
{
   Object& accessObject(int id);
   const Object& getObject(int id) const;

   ...
};

In case you really don't want to have two getters, what about a single const getter where const_cast could be used to cast away constness in case of need ?

codeJack
  • 2,053
  • 3
  • 19
  • 27
  • I guess to getters are the better solution in this case. I think the const_cast would be more messy, wouldn't it? – Cristi Popa Jan 24 '13 at 17:46
  • Yes, I agree, the `const cast` solution was a crappy way of doing it in case you didn't want to duplicate code. The two getter solution is the one I would use. – codeJack Jan 25 '13 at 09:25