Speeding up and shortening a loop [closed]

Question

I'm still relatively new to C++ and I've written a program that could do with being a bit shorter and faster.

I use this same set of for loops many times to cycle through the whole tree of values. I'll use the JSON format to show you a much shortened version of the tree (in reality the tree has another layer of depth with many more values):

{
  "WorldOne": {
    "Earth": {
      "Japan": "Tokyo",
      "America": "New York"
    },
    "Moon": {
      "Dark Side": "Crater One"
    }
  }
}

I then parse this and fill a large vector with these values. The format of the vectors can be seen in the loop below.

My program then, depending on the parameters I set, cycles through to grab the data contained within.

This is the loop I am using over and over again:

for ( auto cWorld : fWorldVec ) //vector of world pointers
        {
            for ( auto cPlanet : ( cWorld )->fPlanetVector ) 
            {
                for ( auto cCity : cPlanet.fCityVector )
                {
                     for (auto cValue : cCity.fValueVector) 
                     {
                          // Then I do stuff
                          // e.g: 
                          ConstructSomething( ( cWorld )->getWorldId(), cPlanet.getPlanetId(), cCity.getCityId(), cValue.getValueId() );
                          // I know the above JSON didn't contain values,
                          // but it does in my code for reasons that aren't worth mentioning
                          // This loop is used so many times throughout my code
                          // It would be wonderful to simply be able to grab the data much faster
                     }
                }
            }
        }

I apologise if this question is too vague, but I really need a speed boost or even to make this loop shorter. I'd like any improvements on the format of my tree or how to condense these loops.

I was thinking along the lines of making some kind of address table with pointers - but I can't quite think of a way to do it.

Answer 1

I cannot think of anything to make it faster.

However, recursion along with some sort of polymorphism and (optionally) visitor pattern can give you the same result without nested loops. Here is an simplified example based on static polymorphism (my code is more abstract than yours but is complete; you might try the same in your question):

template <typename T>
struct vec : std::false_type {};

template <typename T, typename F>
typename std::enable_if <!vec <T>{}>::type
loop(const T& x, F f) { f(x); }

template <typename T, typename F>
typename std::enable_if <vec <T>{}>::type
loop(const T& v, F f)
{
    for (auto &x : v.items())
        loop(x, f);
}

struct A
{
    std::vector <int> v;
    const std::vector <int>& items() const { return v; }
};

struct B
{
    std::array <A, 4> a;
    const std::array <A, 4>& items() const { return a; }
};

template <> struct vec <A> : std::true_type {};
template <> struct vec <B> : std::true_type {};

struct stream
{
    template <typename T>
    void operator()(T&& x) { std::cout << x << " "; }
};

int main()
{
    B b{{{{{1,2,3}}, {{5,6}}, {{7}}, {{8,9}}}}};
    loop(b, stream{});
}

It takes a bit more infrastructure but after that usage is very simple and code is shorter - but not faster.

Type trait vec could be discarded by automatically detecting whether an object has method items() (in which case, loop() recurses down the tree). In this case, the only additional effort in each class would be to define items().

Answer 2

There is no improvement which can be done on the code you show us. If the function constructSomething really needs to be called for each combination of the parameters, there is nothing better you can do.

You should try to optimizie the function, maybe it is possible to cache some part of the construction which is common if one of the parameters is fixed.

For example you can pop the computation cWord->getWorldId() in the outer loop, so that you make it once for every Word (instead of once for every City). But I think this is a slight improvement.