Help with not using the goto statement

[mavericknik]

Hi guys. I'm quite new to c++, and have just written a code that has a few nested loops. To get out of the loops, I'm using the goto statement to get out of the loops at a certain condition. Is there any better way to do this? I have attached, if someone could take a look I'd be grateful.

What i'm trying to do if anyone is interested is write code for a channel routing algorithm. The snippet of code i need checked is

                                                        if (column_map_iterator_b != column_rout_map.end())
							{
								for(int i = 0; i < column_rout_map[b].size()-2; i=i+3)
								{	
									if (track_no <= column_rout_map[b][i+1] && column_rout_map[b][i+2] ==	-2)
									{	
										cout << " A column already exists at the location, moving to next track" << endl;	
										track_no++;
										goto loop3;
									}
								}
			
							}	

if the condition is true, I have to jump outside the loops to the beginning of a while loop, or else I have to continue operation. I could also post the whole code for the function if required. Any help appreciated!

[timmyyyyy]

I would suggest creating a boolean and an "if (boolean is true)" for the rest of your code (after the one you put) ; if it goes to your "goto" part, instead of goto you put the boolean to false, that way it doesnt do anything else and starts again the loop (at the beginning of the loop, you can put the boolean to true again)

[cire]

Not enough code here to give you a definitive answer.

[mavericknik]

Thanks for the suggestion timmyyyy, ill try that out.
cire, here is the code for the whole function.

	while (inserted)
		{
			loop1:			
			cout << "inside while loop" << endl;
			stringstream ss;
			ss << "track_" << track_no;
			key = ss.str();
			cout << "Current key is " << key << endl;
			int q;
			q = row_rout_map_chan1[key].size();
			cout <<"the size of current track: " << key << " is "<< q << endl;
			if (q == 0)
			{
				if (column_map_iterator_a != column_rout_map.end())     			//Checking wether a column already exists at the endpoint of the row.
				{       
					for(int i = 0; i < column_rout_map[a].size()-2; i=i+3)				
					{
						if (track_no <= column_rout_map[a][i+1] && column_rout_map[a][i+2] == -2)
						{	
							cout << " A column already exists at the location, moving to next track" << endl;	
							track_no++;
							goto loop1;
						}
					}
				}
				if (column_map_iterator_b != column_rout_map.end())
				{	
					for(int i = 0; i < column_rout_map[b].size()-2; i=i+3)
					{	
						if (track_no <= column_rout_map[b][i+1] && column_rout_map[b][i+2] == -2)
						{	
							cout << " A column already exists at the location, moving to next track" << endl;	
							track_no++;
							goto loop1;
						}
					}	
				}				
				row_rout_map_chan1[key].push_back(a);
				row_rout_map_chan1[key].push_back(b);
				inserted = 0;
				cout << "The new track was empty, created another vector at track: " << key << " and inserted the values of a & b, which are " << a << " " << b << endl;
				cout << "The trackno is " << track_no << endl;
				return(track_no);			
			}
			else	
			{
				for (long i=0; i< row_rout_map_chan1[key].size(); i=i+2)
				{	
					int c,d;
					c = row_rout_map_chan1[key][i];
					d = row_rout_map_chan1[key][i+1];
					int length_of_track, length_remaining, end;
					end = 50;
					length_of_track = b - a;
					if ( c <= a && a <= d)
					{		
						cout << "We'll have to choose another track." << endl;
						break;
					}
					else if (c <= b && b <= d)
					{	
						cout << "We'll have to choose another track." << endl;
						break;
					}
					else if (c >= a && c <= b)
					{	
						cout << "We choose another track" << endl;
						break;
					}
					else if (i == row_rout_map_chan1[key].size()-2)
					{	
						length_remaining = end - d;
						if (length_of_track > length_remaining)
						{
							cout << "Remaining track length less than required, choosing next track" << endl;
							break;
						}
						else
						{	
							if (column_map_iterator_a != column_rout_map.end())     			//Checking wether a column already exists at the endpoint of the row.
							{       
								for(int i = 0; i < column_rout_map[a].size()-2; i=i+3)				
								{
									if (track_no <= column_rout_map[a][i+1] && column_rout_map[a][i+2] ==	-2)
									{	
										cout << " A column already exists at the location, moving to next track" << endl;	
										track_no++;
										goto loop1;
									}
								}
							}
							if (column_map_iterator_b != column_rout_map.end())
							{	
								for(int i = 0; i < column_rout_map[b].size()-2; i=i+3)
								{	
									if (track_no <= column_rout_map[b][i+1] && column_rout_map[b][i+2] ==	-2)
									{	
										cout << " A column already exists at the location, moving to next track" << endl;	
										track_no++;
										goto loop1;
									}
								}	
							}
							row_rout_map_chan1[key].push_back(a);
							row_rout_map_chan1[key].push_back(b);
							cout << "The value has been succesfully entered into the vector: " << key << endl;	
							inserted = 0;
							return(track_no);
							break;
						}
					break;
					}

				}
			}	
			track_no++;
		}

Thanks for the help.

[cire]

The best advice I could give you is to refactor your code into smaller functions.

The first two goto statements can be directly converted into continue statements.

For the last two I would suggest:

							if (column_map_iterator_a != column_rout_map.end())     			//Checking wether a column already exists at the endpoint of the row.
							{       
								bool column_exists = false ;
								for(int i = 0; i < column_rout_map[a].size()-2; i=i+3)				
								{
									if (track_no <= column_rout_map[a][i+1] && column_rout_map[a][i+2] ==	-2)
									{	
										cout << " A column already exists at the location, moving to next track" << endl;	
										column_exists = true ;
										break ;
									}
								}
								if (column_exists)
									break ;
							}
							if (column_map_iterator_b != column_rout_map.end())
							{	
								bool column_exists = false ;
								for(int i = 0; i < column_rout_map[b].size()-2; i=i+3)
								{	
									if (track_no <= column_rout_map[b][i+1] && column_rout_map[b][i+2] ==	-2)
									{	
										cout << " A column already exists at the location, moving to next track" << endl;	
										column_exists = true ;
										break ;
									}
								}	
								if (column_exists)
									break ;
							}

That gets you out of your nested for loops and track_no will be updated for you at the tail end of the while loop.

[mavericknik]

Thanks cire for the help. I'll certainly try to write more small functions. I understand how using the boolean allows me to bypass the block of code and break out of the loop. But I don't get how I can convert the first two goto's into continues. If i use a continue instead of the goto, wouldn't the loop just increment " i " and keep executing? How would the next block of code be bypassed if the condition is met? Sorry if I'm missing something basic here.

[cire]

Ugh. Somehow I missed that those first two were nested in for loops.


You could handle them like so:

				if (column_map_iterator_a != column_rout_map.end())     			//Checking wether a column already exists at the endpoint of the row.
				{       
					bool column_exists = false ;
					for(int i = 0; i < column_rout_map[a].size()-2; i=i+3)				
					{
						if (track_no <= column_rout_map[a][i+1] && column_rout_map[a][i+2] == -2)
						{	
							cout << " A column already exists at the location, moving to next track" << endl;	
							track_no++;
							column_exists = true ;
							break ;
						}
					}
					if ( column_exists )
						continue ;
				}