Const question

I just wanted to hear opinions on this.
There's a function that takes a pointer...

Code:

---

ERROR1 CRegistry::AddIndex(const CIndex* const pIndex, UINT* const pVectorIndex, const bool bLoadingNow)

---

...which it promises that it will in no way change or mess with.
However, the problem comes later:

Code:

---

vector<CIndex*> m_vIndexes; // Here is where all the indexes are stored
CTypedPtrMap<CMapWordToPtr, UINT64, CIndex*> m_IndexByID; // Used to find an index by its ID

rHive.m_vIndexes.push_back(pIndex);
rHive.m_IndexByID.SetAt(pIndex->dwId, pIndex); // Save index in the list to identify it by its ID

---

The function needs to save the pIndex is a vector for storage and set it in a map for lookup. Other functions will (and must) be able to modify these pointers, but the function itself will not mess around with pIndex.

So the question is: should it be feasible to cast away the const before storing it in the map? That way the function can take a const-type pointer, which be const correct since the function won't modify the pointer.

Perhaps mutable is an option.

This doesn't seem to work since...

Quote:

---

// pIndex == const CIndex* const pIndex
// and the vector takes vector<CIndex*> m_vIndexes
// And gives the compile error error C2664: 'std::vector<_Ty>:: push_back' : cannot convert parameter 1 from 'const CRegistry::CIndex *const ' to 'CRegistry::CIndex *const &'

---

I'm not sure I understand why it's doing that. Mutables can only be modified by const functions and for me it is usually a better alternative in the rare event I need to alter a const.

I will say that using mutable is ugly and I don't normally recommend it because usually it means you need to rethink a design but in this case I would say it would be feasible to use if you could get it to work.

From http://developer.kde.org/~wheeler/cpp-pitfalls.html

Code:

---

class Foo
{
public:
    Foo() : m_object( 0 ) // initalize to null
    {

    }
    ~Foo()
    {
        delete m_object;
    }

    const Object *object() const
    {
        if( !m_object )
            m_object = new Object;

        return m_object;
    }

private:
    mutable Object *m_object;
};

---

If other functions need to modify this I believe they must be const functions.

Sure, but we're not talking about a free object here. We're talking about a vector and a map which takes a pointer of type CIndex* and pIndex in the function is const CIndex* const, so basically you'd be throwing away the const.

Function:

Code:

---

ERROR1 CRegistry::AddIndex(const CIndex* const pIndex, UINT* const pVectorIndex, const bool bLoadingNow) const
{
        // Save index in list
        CHive& rHive = GetHive();
        rHive.m_vIndexes.push_back(pIndex); // ERROR
       
        if (!bLoadingNow) CHECK_IF_ERROR_1( UpdateIndexInfo(pIndex) ); // Update index information in index file
        rHive.m_IndexByID.SetAt(pIndex->dwId, pIndex); // ERROR
        pIndexB = NULL;

        if (pVectorIndex) *pVectorIndex = (rHive.m_vIndexes.end() - rHive.m_vIndexes.begin());
        return ERROR_SUCCESS;
}

---

GetHive() return a CHive as you can see.
Declaration of CHive:

Code:

---

class CHive
{
        friend class CRegistry;
        mutable vector<CIndex*> m_vIndexes;
        vector<CIndex*> m_vFreeBlocks;
        vector<CIndex*> m_vIndexToUpdate;
        mutable CTypedPtrMap<CMapWordToPtr, UINT64, CIndex*> m_IndexByID;
        CIndex* pRoot; // The root index item
        UINT64 nFreeId; // SAVE
        UINT64 nLastOffset; // SAVE
};

---

Code:

---

rHive.m_vIndexes.push_back(pIndex); // ERROR

---

Code:

---

mutable vector<CIndex*> m_vIndexes;

---

Code:

---

ERROR1 CRegistry::AddIndex(const CIndex* const pIndex,

---

I see a problem here. Again I do not use mutable very much at all but if you passed the object as a mutable CIndex* const pIndex perhaps it may work. You would be saying we can modify it but only inside of this function and nowhere else.

I tried this:

Code:

---

#include "stdafx.h"
#include <vector>

class Index
{


};

class Hive
{
    std::vector<Index *> m_vIndexes;

public:

    Hive() { }

    void Add(mutable Index* const pIndex)
    {
        m_vIndexes.push_back(pIndex);
    }

};


int _tmain(int argc, _TCHAR* argv[])
{
    Index *pIndex =  new Index();
    Hive *pHive = new Hive();
   
    pHive->Add(pIndex);

    delete pHive;
    delete pIndex;

    return 0;
}

---

And got this:
e:\msvc 2005 projects\mutabletest\mutabletest.cpp(21) : warning C4042: 'pIndex' : has bad storage class

So I don' think that will work.

Yeah, so question is: should I take a non-const pointer or cast it to non-const to save it in the map (since the function doesn't modify the pointer, only saves it in the map)?
EDIT: I believe the problem lies in that std::vector.push_back() isn't const and doesn't take a const object either, so the compiler refuses to call it.

It will work if you remove the const from in front of CIndex.

Code:

---

void Add(Index* const pIndex)
 {
        m_vIndexes.push_back(pIndex);

}

---

This works too:

Code:

---

class Hive
{
    std::vector<const Index *> m_vIndexes;

public:

    Hive() { }

    void Add(const Index* const pIndex)
    {
        m_vIndexes.push_back(pIndex);

    }

};

---

And yet another:

Code:

---

class Hive
{
    std::vector<mutable Index *> m_vIndexes;

public:

    Hive() { }

    void Add(Index* const pIndex)
    {
        m_vIndexes.push_back(pIndex);

    }

};

---

It depends on what you want to do. It is ok with mutable objects in the vector but it is not ok with passing a mutable object to Add().

Is that the best solution, though? The function isn't supposed to modify pIndex in any way and works fine with const CIndex* const pIndex.

I don't know what the best solution is. All of them are farely ugly to me.

Then I will remove const through a const_cast for now.

In which case you should ask yourself is it necessary for the object to be const in the first place? Either way you look at it you are removing the const. The const_cast method is probably the simplest since you can still pass in a const object and then remove it when and only when you need to.

I don't think any of them are what I would call elegant solutions.

In which case, I will ask another question: is it every necessary for an object to be const? The whole idea of "const correct" is to have the compiler catch bugs for you.
And I'd rather be fully const correct, than half const correct. If the function isn't going to modify the pointer, then the pointer should be fully const.
It's an evil act I have to do, since there's no way out of this situation.

Well your function is not modifying the pointer but push_back obviously is modifying it. So even though you are saying that to the person reading the code that your function does not modify it you are calling a function inside of it that does.

The good part is you only have to cast away const when you do the push_back. The best time for an object to be const is when it is never modified. If you ever need to modify it later then you should probably ask yourself why it is const. Thankfully you can cast it away for those rare times you may need to when no other elegant solution exists.

Your saying that the index pointer cannot be modified and yet later you are saying it must be modifed. This is where I'm losing the logic.

That's kind of silly, really.
push_back isn't const, true, but I don't think it's modifying it, either. It's probably that the function is modifying the class itself (storing the value). However, it takes a type of CIndex* to store, and not const CIndex* const, so the compiler won't allow me.
I need to store it for later since other functions will modify the pointer, so the vector can't store a const pointer either...

The only reason it would take a CIndex * to store and not a const CIndex * const is if it did in fact modify it.

You could do this:


std::vector< const Index *> m_vIndexes;

This tells me that the vector holds constant Index objects. And the compiler likes this as well and does not require you to cast away const later. You can still store const objects but you cannot modify what they point to and you cannot modify the objects via the vector.

So then you are passing pointer to a const object and storing the pointer in the const object vector.
This makes more sense to me. Now when you need to modify the objects later you will run into the same issue. Then you will probably have to const_cast to get it to work.

I don't think the problem is with vector's push_back method, but that you are trying to store a const pointer in a vector that is meant for non-const pointers. But since the pointer is meant to be modifiable, may-be you could drop the const-qualifiers in the add method (or use const_cast - either way you can't protect yourself from errors).

Interesting problem, though.

I agree that the problem is trying to store a const object in a vector meant for non const objects. That's why my std::vector<const index *> version works without changing the Add prototype.

Quote:

---

Originally Posted by Bubba

std::vector< const Index *> m_vIndexes;

This tells me that the vector holds constant pointers to Index objects.

---

Actually that vector holds pointers to constant Index objects, which is presumably what you meant. Many seem to be confused here and may be forgetting to read the type from right to left.

if you only ever intend to call const methods on those Index objects in the vector, then the above is what you should be using, and same in the add function. It's also possible that you may want mutable members in the Index class in this case, but we can't tell without seeing the code.

However if you intend to call non-const methods on those Index objects in the vector at any time, then if should not be a vector of pointers to const, and neither should the declaration of Add.

But as I did explain, other functions need to modify the pointer later, and they get it either from the map or from the vector, both of which the function tries to store the pointer in.
AddIndex is a function that is supposed to prepare and store the pointer in the array and the map.
Remember that is part of a linked list --> so it contains a lot of pointers to walk in it and those need to be updated, for example, which is exactly why I can't store a const CIndex* in the vector or the map.

EDIT: So in essence, you recommend that the AddIndex function should not take a const pointer?

Just for the record I was confused as soon as he posted. :D

>> So in essence, you recommend that the AddIndex function should not take a const pointer?
It can take a const pointer, just not a pointer to const.

In other words, I think AddIndex should take a CIndex* const pIndex rather than a const CIndex* const pIndex. It should not promise to not change the CIndex pointed to by the pointer, because by adding it to a vector that might change it later, it is essentially doing so itself.

OK, here's another one:

Code:

---

CRegistry::CIndex* CRegistry::CreateIndex(const CString& strName, const CIndex::RegType dwType, const DWORD dwTreeLevel, const CIndex* const pParent, const bool bAssignID)

---

CreateIndex takes a constant pointer too because it doesn't modify it.
But:

Code:

---

        CIndex* pIndex;
        pIndex = DEBUG_NEW CIndex;
        // ...
        pIndex->pParent = const_cast<CIndex*>(pParent); // CONST QUESTION
        // ...

---

Here's another one. pIndex->pParent is of type CIndex*.
I guess the same principle applies here. CreateIndex doesn't modify the pointer, but it creates and returns an object that CAN be changed, so I guess I remove const here too?

Yes. If you assign a copy of the original pointer to something else that might modify the object, then that is equivalent to saying that the original pointer might modify the object. So again you should not declare the pointer as referring to a const object.

Got ya.