QHashIterator in c++

Question

I developed a game in C++, and want to make sure everything is properly done. Is it a good solution to use a QHashIterator to check which item in the list has the lowest value (F-cost for pathfinding).

Snippet from my code:

 while(!pathFound){ //do while path is found

        QHashIterator<int, PathFinding*> iterator(openList);
        PathFinding* parent;
        iterator.next();
        parent = iterator.value();

        while(iterator.hasNext()){ //we take the next tile, and we take the one with the lowest value
            iterator.next();
            //checking lowest f value
            if((iterator.value()->getGcost() + iterator.value()->getHcost()) < (parent->getGcost() + parent->getHcost())){
                parent = iterator.value();
            }
        }

        if(!atDestionation(parent,endPoint)){ //here we check if we are at the destionation. if we are we return our pathcost.
            clearLists(parent);
            filllists(parent,endPoint);
        }else{
            pathFound = true;
            while(parent->hasParent()){
                 mylist.append(parent);
                 parent = parent->getParent();
            }
            pathcost = calculatePathCost(mylist);    //we calculate what the pathcost is and return it
        }
     }

If no? Are there better improvements?

I also found someting about the std::priority_queue. It this mutch better then a QHashIterator?

It's maybe not a problem with gameworld where there which are not big. But i'm looking for a suitable solution when the game worlds are big (like + 10000 calculations).Any marks?

Answer 1

Here you basically scan the whole map to find the element that is the minimum one according to some values:

while(iterator.hasNext()){ //we take the next tile, and we take the one with the lowest value
    iterator.next();
    //checking lowest f value
    if((iterator.value()->getGcost() + iterator.value()->getHcost()) < (parent->getGcost() + parent->getHcost())){
        parent = iterator.value();
    }
}

All this code, if you had an stl container, for instance a map, could be reduced to:

auto parent = std::min_element(iterator.begin(), iterator.end(), [](auto& lhs, auto& rhs)
  { lhs.value()->getGcost() + lhs.value()->getHcost()) < (rhs.value()->getGcost() + rhs.value()->getHcost() }

Once you have something easier to understand you can play around with different containers, for instance it might be faster to hold a sorted vector in this case. )

Your code does not present any obvious problems per se, often performance gains are not conquered by optimizing little loops, it's more on how you code is organized. For instance I see that you have a lot of indirections, those cost a lot in cache misses. Or if you have to always find the minimum element, you could cache it in another structure and you would have it at a constant time, all the time.