[SOLVED] Qt Object Manipulation in QMap



  • 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*>


  • Moderators

    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).


  • MyObject* myobject = myObjects[key];
    

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



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


  • Moderators

    * 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.



  • 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++


  • Moderators

    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.



  • Thanks, that works



  • This post is deleted!

Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.