escape multiple nested loops

[fubar]

Hello all,

I have a program that I am unable to rewrite without either

a goto statement
or
using 'return 0;' twice. Once at the end of main(),
and once in the middle of the loops that I want to exit.

Neither is very elegant, but what is generally preferred?

Thanks for your help


Edit: Well, I could use 'exit(0)' instead of the second 'return 0'. But still, the question of elegance remains.

[tomao]

I would guess the best is 'return 0;' in your loop. I have heard bad things abot goto.

[ropez]

Why would you have to exit in the middle of the loop. Would you regard it as an "error" or "exception" from the programs default behaviour. In that case, you could throw an exception.

[matty3269]

what about a break; statement?

[fubar]

I want to search for a text-fragment (fragment[]) in an input line (line[]) , which are both read from the console, and declared as the same maximal size.
FragmentLength is the number of non-zero elements in array 'fragment'.
In the end, the program should print the number of the element in the array 'line', where 'fragment' begins.

 for (i=0; i < LineLength; i++)   // Check for each of the i elemets
      {                                // of 'line'
	if (line[i] == fragment[0])     // if the first element of fragment
	  {                            // matches the i-th Element of 'line'.
	    for (int j=0; j < FragmentLength; j++)   
	      {
		if (line[i+j] == fragment[j] && (j+1 == FragmentLength))  
		    {                                
		      cout << "Index is: " << i << endl;   
		      exit(0);   //  Or:  goto exit;
		    }	
	      }
	  }
      }
    
    // exit: 
    if (i == LineLength)
      cout << "Your fragment could not be found in line.\n";
      
    return 0;

You see, there would be (too) *many* if/break statements necessary.
The code is fully functional, though inelegant.

[ropez]

What about this:

int search(const char* line, int LineLength, const char* fragment, int FragmentLength)
{
    for (i=0; i < LineLength; i++)   // Check for each of the i elemets
    {                                // of 'line'
        if (line[i] == fragment[0])     // if the first element of fragment
        {                               // matches the i-th Element of 'line'.
            for (int j=0; j < FragmentLength; j++)   
            {
                if (line[i+j] == fragment[j] && (j+1 == FragmentLength))  
                {                                
                    return i; // Found
                }	
            }
        }
    }
    return LineLength; // Not found
}


int main()
{
    // ...
    index = search(line, LineLength, fragment, FragmentLength);
    if (index < LineLength)
    {
        cout << "Index is: " << index << endl;
    }
    else
    {
        cout << "Your fragment could not be found in line.\n";
    }
    // return 0; // Not really necessary
}

[ropez]

BTW: Your code it actually wrong. In the outermost loop, you should replace the condition with:

for (i=0; i < LineLength - FragmentLength; i++)

Otherwise, you will read outside the end of the line.

Also, this test isn't necessary:

if (line[i] == fragment[0])

Because the exact same condition will be checked in the first iteration of the inner loop. It is actually a bug to have this test, because what if FragmentLength is 0...

[gzero]

I've only skimmed the above posts (other than the original), but here is a way to break out of a deeply nested loop without use of a goto statement. Just try running it, and you'll see that after the condition is met in the innermost loop, it jumps right out past all the others.

#include <iostream>
using namespace std;

int main()
{
    bool flag = false;
    int a = 0;
    int b = 0;
    int c = 0;
    for(int i=0; i <100; ++i)
    {
       a++;
       for (int n =0; n<50; ++n)
       {
           b++;
           for (int x = 0; x <25; ++x)
           {
               if (x==14)
               {
                 flag = true;
                 break;
               }
               ++c;
           }
           if (flag==true){break;}
       }
       if (flag==true){break;}
    }
    cout<<"a: "<<a<<endl;
    cout<<"b: "<<b<<endl;
    cout<<"c: "<<c<<endl;
    system ("pause");
}

[fubar]

Thank you guys very much for your contributions!

Though, I dont't think I should replace the outermost loop's condition
for (i=0; i < LineLength; i++)
with
for (i=0; i < LineLength - FragmentLength; i++)
Please consider the following example:
LineLength = 17
FragmentLength = 3
Suppose the fragment I am looking for begins at element #14, so its last element is in element #16 of 'line'. However, you suggest I should only read up to 17-3, that is 14. I get an error.

Also, you are saying that this test isn't necessary:

if (line[i] == fragment[0])

When I omit this test, I keep getting errors when I try to find a 'fragment' that is towards/at the end of 'line'. Why?
My code handles this cases correct. I inserted this test because I only wanted the program to descend into the subsequent loops if this condition was met. I thought the program would be faster like that.

And, if FragmentLength is 0, well, the "fragment could not be found in line."

Can anyone help?

[ropez]

If you don't replace the loop, your program will read outside the end of the string, which is a serious bug.

If you don't remove the test, and FragmentLength is 0, your program will read outside the end of the fragment string, but it's only one character which is always \0 anyway, so it's not serious. However, it does not make your code any faster, just more complicated, I can assure you that.

[fubar]

Hi ropez, thanks for your immediate response.

Maybe I wasn't specific enough and there is a misunderstanding.
I mean, LineLength represents the actual length of the line, something like: LineLength = strlen(line);
So I still believe the program won't read outside the end of the string.


I must admit, when I just look at the program, the test seems redundant. But how can you explain the wrong indices when I omit it?

[ropez]

LineLength = strlen(line);
FragmentLength = strlen(fragment);
i = LineLength - 1;
j = FragmentLength - 1;
line[i + j]; // Outside the end of line 

If your line is 17 characters, and your fragment is 3 characters, you don't need to search more than 14 characters, because you can't have a three characgter segment starting at position 15 in a 17 character string.

For the if-test, consider this test when j == 0:

line[i+j] == fragment[j]
// equals
line[i] == fragment[0]

It's exactly the same test inside the loop as you have outside.

There are other errors that cause your "wrong indices". If a character isn't equal in the fragment and the line, besides the last character in the fragment. Your code should conclude that the fragment doesn't match the current position, and move on. But it doesn't. It just checks the next character. Maybe you want to change the inner loop to this (from the version I posted):

if (line[i+j] != fragment[j])
    break; // Move to next position
else if (j+1 == FragmentLength)
    return i; // Found 

[z05DSL3A]

Just a coment on gzero's post:

It would probably be better to put the check on ‘flag’ as a condition for the for loop: (I’ve renamed flag to make the code scan better).

    bool _continue = true;
    int a = 0;
    int b = 0;
    int c = 0;

    for(int i=0; i <100 && _continue ; ++i)
    {
       a++;
       for (int n =0; n<50 && _continue ; ++n)
       {
           b++;
           for (int x = 0; x <25; ++x)
           {
               if (x==14)
               {
                 _continue  =  false;
                 break;
               }
               ++c;
           }           
       }
    }

Edited due to human error

[ropez]

@Grey Wolf:
You can't use "continue" as a variable name, because it's a reserved keyword in C++. Besides, you need to use && instead of ||.

[z05DSL3A]

Thanks Ropez,
I normally use an underscore in front of my variable names so it should have been _continue but as I was editing someone else’s code I forgot it. Don’t know why I used || instead of &&, I think I had my head stuck in a SQL trigger that I was taking a break from.

[fubar]

Well, thank you very much for your help, ropez!

The only thing that I had to change in your suggestion was adding the '+1' in the outermost loop:
for (i=0; i < LineLength - FragmentLength +1; i++)
That was preventing the program to run correctly in cases where you are searching for a 'fragment' that incorporates (or consists only of) the very last (non-null) character of 'line'.

Now it's running correctly, as far as I can see!
Cheers mate!