30

I have a class of objects and need to compare one property of each object to the same property of all other objects. If they match, the code needs to do something. This results in two 'for loops' looping through the objects to get that property, and in the second 'for loop', there is a third 'for loop' going through the elements of the property (which is a vector) to compare them. If they match, I need the outer most 'for loop' to abort the current iteration and go on to the next one (I only want the first match with another object to be considered).

I have looked into 'goto' statements and into creating a do{}while() structure, but have not been able to implement them in a way to get the desired result. What I need is something like a 'continue' statement for the outer most loop based on what happens in the conditional statement in the inner most loop.

Which would be a good method to achieve this, and how would it have to be implemented?

Edit: Next to the answer I accepted, I would also recommend Martin Bonner's answer, which also works perfectly fine and doesn't rely on goto.

for (int i = 0; i < max; i++){
Object & object1 = system.getAgent(i);
VectorOfStrings object_property1 = object1.getProperty();

    for (int j = i + 1; j < max; j++){
    Object & object2 = system.getObject(j);
    VectorOfStrings object_property2 = object2.getProperty();

        for (unsigned int k = 0; k < object_property1.size(); k++){

            if (object_property1[k] == object_property2[k]){

            //do something

            break; //this aborts the inner most loop
            //Additionally, I need the outer most loop to move on one iteration
            }
        }
    }
}

So if the conditional statement in the 'k' loop is met, I want the 'i' loop to abort the current iteration and move on to the next one.

Also, since I'm new, the code might be inelegant, but please focus on this specific problem in your answers! Unless a general restructuring of the code might be the solution to the problem of course :)

faranzki
  • 403
  • 1
  • 4
  • 6
  • *I need the outer most loop to move on one iteration* you mean increment i? Also have you though of what happens if object_property1 doesn't have the same size as object_property2? – stijn Dec 16 '16 at 07:54
  • Aside: Until I got to "I lack the vocabulary ... in English", I had assumed you were a native English speaker like me. Your English is fine. – Martin Bonner supports Monica Dec 16 '16 at 08:02
  • The `do {...} while(false);` hack works for avoiding nested if-else's not nested loops, because it also uses `break` to jump out of the switch. – stefaanv Dec 16 '16 at 08:20
  • @stijn yeah but they are of the same size because they are members of the same class so that doesn't create problems! – faranzki Dec 16 '16 at 08:22
  • @stefaanv thank you for that info! – faranzki Dec 16 '16 at 08:23
  • @faranzki sure that might be the case now but in programming it is usually better to be defensive about that. If the 'they are the same' condition ever changes you'd have to update all dependent code. So if you take care of this now, either via an assert or by calculating the max index, your code is safe. – stijn Dec 16 '16 at 09:10

4 Answers4

52

This may be implemented with goto:

for (int i = 0; i < max; i++){
    Object & object1 = system.getAgent(i);
    VectorOfStrings object_property1 = object1.getProperty();

    for (int j = i + 1; j < max; j++){
        Object & object2 = system.getObject(j);
        VectorOfStrings object_property2 = object2.getProperty();
        for (unsigned int k = 0; k < object_property1.size(); k++){
            if (object_property1[k] == object_property2[k]){
                //do something
                goto cnt; //this aborts the inner most loop
                //Additionally, I need the outer most loop to move on one iteration
            }
        }
    }
    cnt:;
}

This is one of the rare cases when usage of goto really simplifies code.

alexeykuzmin0
  • 6,344
  • 2
  • 28
  • 51
  • 4
    I would make the label have a meaningful name like `property_matched`. – Martin Bonner supports Monica Dec 16 '16 at 08:08
  • 1
    @MartinBonner I meant `cnt` to be meaningful as an acronym from `continue` – alexeykuzmin0 Dec 16 '16 at 11:44
  • @alexeykuzmin0 that's still not clear, since it can't be continue itself, make it something like `goto_continue` also **note:** ***curly brackets are mandatory for this to work***, otherwise it breaks variable scoping for some reason x) – jave.web Dec 04 '20 at 18:21
  • 4
    **Don't use `goto` unless absolutely absolutely necessary - you may know what you're doing, but it's a matter of time until someone other comes in and uses it to completely break your application and possibly destroy data!** – jave.web Dec 04 '20 at 18:22
9

The most readable way to solve this classic problem is almost always to put the nested loops in a function. I don't know what the specific code is suppose to do, but here is some pseudo code you can base a solution on:

bool keep_going = true;
for (int i = 0; i < max && keep_going; i++)
{
  Object& object1 = system.getAgent(i);
  keep_going = !compare_objects(object1.getProperty(), i+1, max);
}

where compare_objects is something like this:

inline bool compare_objects (const VectorOfStrings& obj_prop1, size_t begin, size_t end)
{
  for(size_t i=begin; i<end; i++)
  { 
    Object& obj2 = system.getObject(j);
    VectorOfStrings obj_prop2 = obj2.getProperty();

    for size_t j = 0; j < obj_prop1.size(); j++)
    {
      ifobj_prop1[j] == obj_prop2[j])
      {
        do_something();
        return true;
      }
    }
  }

  return false;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
7

I strongly recommend @alexeykuzmin0's approach, but if you have to work in an environment where goto is forbidden, then the alternative (with annoying flag) is:

for (int i = 0; i < max; i++){
    Object & object1 = system.getAgent(i);
    VectorOfStrings object_property1 = object1.getProperty();

    bool property_matched = false;
    for (int j = i + 1; j < max && !property_matched; j++){
        Object & object2 = system.getObject(j);
        VectorOfStrings object_property2 = object2.getProperty();
        for (unsigned int k = 0; k < object_property1.size(); k++){
            if (object_property1[k] == object_property2[k]){
                //do something
                property_matched = true; // This will break the "j" loop
                break; //this aborts the inner most loop
            }
        }
    }
}
  • 1
    I don't think in this case it is annoying, since you probably do want to know if the property was matched. – stefaanv Dec 16 '16 at 08:16
  • 1
    @stefaanv: Yes, if you want to emulate something like the python `for ... else ...` then the flag is useful. On the other hand, in the label case, you can just put the code that handles "didn't match" directly after the middle for (possibly with a continue), and the code that handles "match" after the label. – Martin Bonner supports Monica Dec 16 '16 at 08:47
  • 1
    @MartinBonner one more question: Whilst this works fine, I get a compiler warning: "warning: label ‘property_matched’ defined but not used". How can I avoid this? – faranzki Dec 16 '16 at 08:55
  • 1
    Ah "*label* property_matched not used". If you are using the flag, just remove the label! You certainly don't want both. – Martin Bonner supports Monica Dec 16 '16 at 09:01
  • 1
    Silly me. I overlooked it when switching from the label option to the flag option and forgot to delete. Please ignore that stupid comment. – faranzki Dec 16 '16 at 09:12
7

You can use lambda and use return stament to change the control. If you return true the lambda will end and outter "for" will "continue".

Compact version:

for(;;) {
    [&]() {
        for(;;) {
            if(/*break condition here*/)
                return;
        }
    }();
}

A more general template:

for(;;) {
    auto innerLoop = [&]() {
        for(;;) {
            if(/*break condition here*/)
                return true;
        }
        return false;
    };

    if(innerLoop())
        continue;
}

For this case:

for (int i = 0; i < max; i++){
    Object & object1 = system.getAgent(i);
    VectorOfStrings object_property1 = object1.getProperty();

    auto innerLoop = [](){
        for (int j = i + 1; j < max; j++){
        Object & object2 = system.getObject(j);
        VectorOfStrings object_property2 = object2.getProperty();

            for (unsigned int k = 0; k < object_property1.size(); k++){

                if (object_property1[k] == object_property2[k]){
                    //do something
                    return true;
                }
            }
        }
        return false;
    };

    if(innerLoop())
        continue;
}
Bediver
  • 692
  • 6
  • 11
  • 31
    clever, but about as readable as an ancient cuneiform tablet that has been carved by a dyslexic scribe who was high on mushrooms at the time. – Richard Hodges Dec 16 '16 at 08:20
  • I didn't test this because I'm a bit confused as to how to implement this based on the code you provided! Am I supposed to leave the brackets after the if() empty? – faranzki Dec 16 '16 at 08:33
  • It's just a template, put condition to break into that if() – Bediver Dec 16 '16 at 08:35
  • Actually, in the particular case of the OP, the lambda doesn't need to return 'true' or 'false'. Looking at your code, you can see that just `innerLoop()` will work just fine. – Martin Bonner supports Monica Dec 16 '16 at 08:38
  • 1
    An alternative would be, if the innerloop lambda could have a meaningful name and could be reused, to move it to a regular function. – stefaanv Dec 16 '16 at 08:47
  • It seems that I cannot use 'auto'. These are the compiler messages: warning: ‘auto’ changes meaning in C++11; please remove it error: ‘innerloop’ does not name a type error: ‘innerloop’ was not declared in this scope – faranzki Dec 16 '16 at 08:52
  • 2
    This is just pointless meta programming, it doesn't increase readability the slightest. In fact, it does the opposite. – Lundin Dec 16 '16 at 08:53