[SOLVED] Qt Object Manipulation in QMap

Maxim DC

This means im accessing a location im not allowed to, but i dont know whats the problem.

void MyClass::updateMyObject(int a, int b, int c, int d)
{
    MyKey* key= new myKey(a, b, c);
    MyObject* myobject = myObjects[key];
    if (d)
    {
        //myobject->mD = true;
    }
    else
    {
        //myobject->mD = false;
    }
    qDebug() << "Updating A:" << a << "B:" << b << "C:" << c  << "to" << d;
}

When uncommenting myobject->mD = ..., it throws an error in the debugger or crashes when running
PS: myObjects is a QMap<MyKey*, MyObject*>

Chris Kawa

This code has a couple of varying degree problems:

you're never releasing the key so you're leaking memory
why even allocate the key on the heap? If you only need it for lookup it would be a lot easier, faster and less error prone to allocate it on the stack.
if myObjects is a QMap<MyKey, MyObject> then how is that even compiling if key is a MyKey*?
if myObjects is a QMap<MyKey, MyObject> then how is that even compiling if you assign a value from it to MyObject*?
you're never checking if myObject is valid. The map operator[] inserts a default-constructed value into the map if it can't find the key so you might be getting a null pointer back, in which case trying to access it via myobject->mD will obviously crash.
not technically an error but doing a boolean test on an int value is a bad style. Be explicit and state your intent clearly: if(d != 0).

zool

MyObject* myobject = myObjects[key];

If your container is QMap<MyKey, MyObject> so operator[] will return MyObject& but not MyObject*. Am i right?

Maxim DC

@zool im sorry, qt forum reads a pointer as a list item, edited the post

Chris Kawa

* is a special character. Please put code in `` or it will swallow these characters: QMap<MyKey*, MyObject*>.

Anyway, most of my points stand - you're leaking memory and not checking if the value returned from operator[] is not null.

Maxim DC

Thanks ill try it, and can you please refer me to a good page of qt and pointers because im a bit new to c++

Chris Kawa

The problem here is that you use a pointer as a key, which is a bad idea. If you create a new key every time for a lookup it will never find that value because the new pointer has a different value.

The best would be to use a value, not a pointer as a key:

QMap<MyKey, MyObject*> myObjects; //use a value as a key, not a pointer

void MyClass::updateMyObject(int a, int b, int c, int d)
{
    MyKey key(a, b, c);
    auto it = myObjects.find(key);   //doesn't insert empty values when not found
    if(it != myObjects.end())        //check if the item was actually found
    {
        (*it)->mD = (d != 0);        //it is an iterator, (*it) is a MyObject*
    }
}

Or, if the map really needs a pointer key, then you need to use that pointer as a key, not create a new one:

QMap<MyKey*, MyObject*> myObjects; //use a value as a key, not a pointer

void MyClass::updateMyObject(MyKey* key, int d)
{
    auto it = myObjects.find(key);   //doesn't insert empty values when not found
    if(it != myObjects.end())        //check if the item was actually found
    {
        (*it)->mD = (d != 0);        //it is an iterator, (*it) is a MyObject*
    }
}

I haven't used C++ tutorials in quite some time, but cplusplus.com usually has good material. This is not a Qt specific topic so you can find anything about std::map or std::vector and the info will still be valid, as Qt containers have very similar interface and properties.

Maxim DC

Thanks, that works

ChrisW67

This post is deleted!