Recent

Author Topic: Eliminating Goto/Label  (Read 5397 times)

MaxCuriosus

  • Full Member
  • ***
  • Posts: 136
Eliminating Goto/Label
« on: July 17, 2020, 12:02:43 pm »
In the code skeleton below I'd like to eliminate the gotos/labels while maintainig a reasonably equivalent clarity in the "flow chart" of the code. How could I do that?

Code: Pascal  [Select][+][-]
  1. Procedure Skeleton;
  2. Label
  3.   Reload, DispLoop, Lexit;
  4.  
  5. Begin
  6.   { Code #1 }
  7.   If Condition1 then
  8.     Begin
  9.       { Code #2 }
  10.       Goto Lexit;
  11.     end;
  12.   { Code #3 }
  13.  
  14. Reload:
  15.   { Code #4 }
  16.  
  17. DispLoop:
  18.   { Code #5 }
  19.  
  20.   If Condition2 then Begin Goto DispLoop; end;
  21.  
  22.   If Condition3 then
  23.  
  24.     Begin
  25.  
  26.       Case VK of
  27.         0:
  28.           Begin Goto Lexit; end;
  29.         1:
  30.           Begin
  31.             If Condition4 then Begin Goto DispLoop; end;
  32.             { Code #6 }
  33.             Goto Reload;
  34.           end;
  35.         2:
  36.           Begin
  37.             { Code #7 }
  38.             Goto Reload;
  39.           end;
  40.         3:
  41.           Begin
  42.             { Code #8 }        
  43.             If Condition5 then
  44.               Begin
  45.                 { Code #9 }
  46.                 Goto DispLoop;
  47.               end;
  48.             { Code #10 }
  49.             Goto Reload;
  50.           end;
  51.         4:
  52.           Begin
  53.             { Code #11 }
  54.             If Condition6 then Begin Goto DispLoop; end;
  55.             If Condition7
  56.               then
  57.                 Begin
  58.                   { Code #12 }
  59.                   Goto Reload;
  60.                 end
  61.               else
  62.                 Begin
  63.                   Goto DispLoop;
  64.                 end;
  65.           end;
  66.         5:
  67.           Begin
  68.             { Code #13 }
  69.             Goto DispLoop;  
  70.           end;
  71.  
  72.       end; {Case}
  73.  
  74.     end; {If Condition3}
  75.  
  76. Lexit:
  77.   { Code #14 }
  78. end;
  79.  

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11383
  • FPC developer.
Re: Eliminating Goto/Label
« Reply #1 on: July 17, 2020, 12:18:07 pm »
One to one substitutions in cases of multiple exit/break are hard. I did give it a try to solve it without additional state variables:

Code: Pascal  [Select][+][-]
  1.     Procedure Skeleton;
  2.     Label
  3.       Reload, DispLoop, Lexit;
  4.      
  5.     Begin
  6.      try
  7.       { Code #1 }
  8.       If Condition1 then
  9.         exit; // if lexit is always exit procedure.
  10.       { Code #3 }
  11.      
  12.     Reload:
  13.       { Code #4 }
  14.       while true do
  15.         begin
  16.      
  17.     DispLoop:
  18.       { Code #5 }
  19.       while true do
  20.         begin
  21.  
  22.       If Condition2 then
  23.           continue;
  24.      
  25.       If Condition3 then
  26.    
  27.         Begin
  28.      
  29.           Case VK of
  30.             0:
  31.               exit;
  32.             1:
  33.               Begin
  34.                 If Condition4 then continue;
  35.                 { Code #6 }
  36.                 break;
  37.               end;
  38.             2:
  39.               Begin
  40.                 { Code #7 }
  41.                 break;
  42.               end;
  43.             3:
  44.               Begin
  45.                 { Code #8 }        
  46.                 If Condition5 then
  47.                   Begin
  48.                     { Code #9 }
  49.                      continue;
  50.                   end;
  51.                 { Code #10 }
  52.                 break;
  53.               end;
  54.             4:
  55.               Begin
  56.                 { Code #11 }
  57.                 If Condition6 then continue;
  58.                 If Condition7
  59.                   then
  60.                     Begin
  61.                       { Code #12 }
  62.                       break;
  63.                     end
  64.                   else
  65.                     Begin
  66.                       continue;
  67.                     end;
  68.               end;
  69.             5:
  70.               Begin
  71.                 { Code #13 }
  72.                 continue;
  73.               end;
  74.      
  75.           end; {Case}
  76.      
  77.         end; {If Condition3}
  78.         break; // if we get here, there is no terminating goto, so assume no loop.
  79.                // it could be that that we never get here (there is always a break or continue), in that case, kill the continues, and this break
  80.        end; {disploop}
  81.        // we get here for "breaks", this, run reloadloop again.
  82.        end {reloadloop}
  83.       finally
  84.       { Code #14 }
  85.        end;
  86.     end;
  87.      

Warfley

  • Hero Member
  • *****
  • Posts: 1499
Re: Eliminating Goto/Label
« Reply #2 on: July 17, 2020, 03:05:12 pm »
Replacing goto simply for the replacements sake is generally a bad idea, goto is not bad because there is something inherently bad in using this keyword, but because it leads to poorly designed code.

Meaning a 1-1 substitution, while getting rid of goto, will not get rid of the design of the code, and therefore does not improve code quality.

The clearest example for this I can think of:
Code: Pascal  [Select][+][-]
  1. label foo;
  2. begin
  3.   ...
  4.   goto foo;
  5.   ...
  6. foo:
  7.   ...
can be replaced with:
Code: Pascal  [Select][+][-]
  1. type TGotoFooException = class;
  2. begin
  3.   try
  4.     ...
  5.     raise TGotoFooException.Create;
  6.     ...
  7.   except on e: TGotoFooException do;
  8.   end;
  9.   ...
I think it is pretty obvious that the second code, even though it does not contain goto is actually worse in every way.

So I am also not really fond of the way it was done by marcov, because the code does not really get more readable with these while true, continue-break approaches, because this is basically the same jumping around, just less obvious.

IMHO you should change the design of that function a bit to use the full extend of the tools available to you. These are:
1. Try-Finally to allow you to jump to a final code block that should be executed in the end, meaning together with exit() this can be used as "jump to end"
2. If blocks let you jump over blocks depending on a condition, which can basically express a jump down to
3. repeat until lets you jump back to the beginning of a block if a condition is true, which basically expresses a jump up to
4. While basically combines the features of if and repeat-until
5. Functions allow you to call code blocks by name independently of the current position in code.

Your function does basically the following:
1. if a certain condition is fulfilled, simply skip the whole function
2. execute some code (reload, disploop)
3. with the results of step 2 do other code and either decide if you are finished, need to start over at 2 or start over at 2 but skip the reloading.
4. on finish execute some final code (even if step 1 skip is done)

First the obvious the 4. point can be realized with a try-finally and the first point with the exit "function".
Code: Pascal  [Select][+][-]
  1. try
  2.   if Condition1 then
  3.     Exit;
  4.   // LOOPCode
  5. finally
  6.   Code14
  7. end;

So let's look into the loop code.
First and foremost it is always a good idea to put code in separate functions to have concise and readable functions (use nested functions to not fill up your scope with one-time use functions), so for all your Code #N comments I will simply assume that there is a function for it called CodeN

The next thing is, Reload and DispLoop belong together, if Reload is executed disploop will always be also executed. So Reload is optional and disploop is mandatory. There are two ways to implement this, explicitly skip Reload or explicitly execute Reload.
Because the number of Reloads is not so significantly different from the number of disploops without reloads, I think the best option would be to explicitely skip reload so we can simply do this function:
Code: Pascal  [Select][+][-]
  1. procedure DispLoop(reload: boolean);
  2. begin
  3.   if Reload then // (explicit) skip of reload if not requested
  4.     Code4; // reload section
  5.   Code5; // disploop section
  6. end;

Your loop then can be expressed by a repeat-until (as you want it to run at least once and multiple times if requested). Whether you want to execute reload or not needs to be saved in some form of state variable:
Code: Pascal  [Select][+][-]
  1. repeat
  2.   DispLoop(doReload); // function defined above
  3.   // Condition 2
  4.   if Condition 3
  5.     // case statement
  6. until Condition3 Or VK = 0;

The two loop breaking cases Condition 3 and VK = 0 are already covered by the loop condition, so if you read your code you will now see on a first glance what the stop condition is (which is much more complicated if it is a break or continue or goto buried somewhere deep in your code).

For condition2 you have two possibilities, using continue, which will directly jump to the beginning of the loop again, or to combine condition 3 and 2:
Code: Pascal  [Select][+][-]
  1. // first option:
  2. repeat
  3.   DispLoop(Reload); // function defined above
  4.   Reload = false;
  5.   if Condition2 then Continue;
  6.   if Condition 3
  7.     // case statement
  8. unti notl Condition3 Or VK = 0;
  9. // Second option
  10. repeat
  11.   DispLoop(Reload); // function defined above
  12.   if not Condition2 and Condition 3
  13.     // case statement
  14. until (not Condition2 and Condition3) Or VK = 0;
Personally I think the continue call is cleaner if you follow my advice and encapsulate the DispLoop and reload functionality into another function (and therefore the loop will be pretty slim). If you decide to pack everything in here and create a monster loop, I think that the second approach will be better because otherwise the continue will be buried in the code and not be visible that simple

So finally your case statement reduces to setting the reload variable
Code: Pascal  [Select][+][-]
  1. case VK of
  2.   1:
  3.   begin
  4.     Reload := not Condition4;
  5.     if Reload then
  6.       Code6;
  7.   end;
  8.   2:
  9.   begin
  10.     Reload := true;
  11.     Code7;
  12.   end;
  13.   3:
  14.   begin
  15.     Code8;
  16.     Reload := not Condition5;
  17.     if Reload then
  18.       Code10
  19.     else
  20.       Code9;
  21.   end;
  22.   4:
  23.   begin
  24.     Code11;
  25.     Reload := False;
  26.     if not Condition6 and Condition7 then
  27.     begin
  28.       Code12;
  29.       Reload := True;
  30.     end;
  31.   end;
  32.   5:
  33.   begin
  34.     Code13;
  35.     Reload := False;
  36.   end;
  37. end;
Of course how the conditions inside the if are built can be up to you (I like only setting reload an then branching over reload)

Putting it all together
Code: Pascal  [Select][+][-]
  1. var
  2.   Reload: Boolean = true;
  3. begin
  4.   try
  5.     if Condition1 then Exit;
  6.     repeat
  7.       DispLoop(Reload);
  8.       Reload := False;
  9.       If Condition2 then Continue;
  10.       if Condition3 then
  11.         case VK of
  12.         1:
  13.         begin
  14.           Reload := not Condition4;
  15.           if Reload then
  16.             Code6;
  17.         end;
  18.         2:
  19.         begin
  20.           Reload := true;
  21.           Code7;
  22.         end;
  23.         3:
  24.         begin
  25.           Code8;
  26.           Reload := not Condition5;
  27.           if Reload then
  28.             Code10
  29.           else
  30.             Code9;
  31.         end;
  32.         4:
  33.         begin
  34.           Code11;
  35.           Reload := False;
  36.           if not Condition6 and Condition7 then
  37.           begin
  38.             Code12;
  39.             Reload := True;
  40.           end;
  41.         end;
  42.         5:
  43.         begin
  44.           Code13;
  45.           Reload := False;
  46.         end;
  47.       end;
  48.     until not Condition3 or (VK=0);
  49.   finally
  50.     Code14;
  51.   end;
  52. end;
« Last Edit: July 17, 2020, 04:50:28 pm by Warfley »

440bx

  • Hero Member
  • *****
  • Posts: 3944
Re: Eliminating Goto/Label
« Reply #3 on: July 17, 2020, 03:07:27 pm »
In the code skeleton below I'd like to eliminate the gotos/labels while maintainig a reasonably equivalent clarity in the "flow chart" of the code. How could I do that?
The _correct_ way to do that is to rethink the structure of the program to find one that doesn't require producing spaghetti code.

Here is a "solution" that is _slightly_ less undesirable because it doesn't use any "goto"s but, in spite of not using any "goto"s, it is still a form of spaghetti code. 
Code: Pascal  [Select][+][-]
  1. program TestNoGoto;
  2.  
  3.   procedure LasagnaCode;
  4.   var
  5.     Condition1 : boolean = false;
  6.     Condition2 : boolean = false;
  7.     Condition3 : boolean = false;
  8.     Condition4 : boolean = false;
  9.     Condition5 : boolean = false;
  10.     Condition6 : boolean = false;
  11.  
  12.  
  13.     Reload     : boolean = false;    { pasta  layer                           }
  14.     DispLoop   : boolean = false;    { cheese layer                           }
  15.  
  16.     Scope      : integer = 1;        { used to create an inline scope         }
  17.  
  18.  
  19.   begin
  20.     { no inline scopes in Pascal, "synthesize" one                            }
  21.  
  22.     for Scope := Scope to Scope do   { scope to break out of                  }
  23.     begin
  24.       if Condition1 then
  25.       begin
  26.         break;                  { empty plate : no lasagna today              }
  27.       end;
  28.  
  29.       Reload := true;           { Reload loop control variable                }
  30.       while Reload do
  31.       begin
  32.  
  33.         DispLoop := true;       { DispLoop control variable                   }
  34.         while DispLoop do
  35.         begin
  36.  
  37.           if Condition2 then
  38.           begin
  39.             DispLoop := false; { end DispLoop and continue Reload loop        }
  40.  
  41.             break;             { out of DispLoop                              }
  42.           end;
  43.  
  44.           { NOTE: better than testing for Condition3 and nesting a "case"     }
  45.           {       statement in it, would be to test for "not condition3" and  }
  46.           {       simply exit, thus:                                          }
  47.           {                                                                   }
  48.           {       if not Condition3 then                                      }
  49.           {       begin                                                       }
  50.           {         DispLoop := false;                                        }
  51.           {         Reload   := false;                                        }
  52.           {                                                                   }
  53.           {         break;                                                    }
  54.           {       end;                                                        }
  55.  
  56.           if Condition3 then
  57.           begin
  58.             case Vk of
  59.               0 : begin
  60.                     Reload   := false;        { end pasta                     }
  61.                     DispLoop := false;        { end cheese                    }
  62.  
  63.                     break;
  64.                   end;
  65.  
  66.               1 : begin
  67.                     if Condition4 then
  68.                     begin
  69.                       continue;          { start new DispLoop iteration       }
  70.                     end;
  71.  
  72.                     DispLoop := false;   { start a new Reload iteration       }
  73.                     break;
  74.                   end;
  75.  
  76.               2 : begin
  77.                     DispLoop := false;   { force DispLoop to terminate        }
  78.  
  79.                     break;               { start a new Reload iteration       }
  80.                   end;
  81.  
  82.               3 : begin
  83.                     if Condition5 then
  84.                     begin
  85.                       continue;          { start new DispLoop iteration       }
  86.                     end;
  87.  
  88.                     DispLoop := false;   { start a new Reload iteration       }
  89.                     break;
  90.                   end;
  91.  
  92.               4 : begin
  93.                     if Condition6 then
  94.                     begin
  95.                       continue;          { start new DispLoop iteration       }
  96.                     end;
  97.  
  98.                     if Condition7 then
  99.                     begin
  100.                       DispLoop := false; { start a new Reload iteration       }
  101.                       break
  102.                     end;
  103.                   end;
  104.  
  105.               5 : begin
  106.                     continue;            { start new DispLoop iteration       }
  107.                   end;
  108.  
  109.               { it would be a good idea to have an "otherwise" for safety     }
  110.  
  111.               otherwise
  112.               begin
  113.                 { whatever should be done in this case                        }
  114.  
  115.                 ;          { do nothing - meditate over infinite lasagna      }
  116.               end;
  117.             end; { case }
  118.  
  119.           end; { Condition3 }
  120.  
  121.         end; { DispLoop }
  122.  
  123.       end; { Reload loop }
  124.  
  125.     end; { for scope }
  126.  
  127.     { do whatever here - surgeon general warning                              }
  128.  
  129.     writeln('Abuse of lasagna and spaghetti are not healthy');
  130.   end;
  131.  
  132.  
  133. begin
  134.   LasagnaCode();
  135. end.
  136.  
Neither the code above nor the code you showed is desirable in any way.  There is a better solution, the first step in finding it is to stop coding and rethink the structure of the program.

HTH.

(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

MarkMLl

  • Hero Member
  • *****
  • Posts: 6676
Re: Eliminating Goto/Label
« Reply #4 on: July 17, 2020, 04:15:36 pm »
I'd echo in particular what @Warfley says about try/exit/finally and repeat/continue/break/until.

However I'd suggest that the bottom line is that "clever" code intended to conform to some bit of trendy dogma might actually be more difficult to follow than the alternative "naughty" form.

I'm fairly relaxed about goto, there are cases where it's quite simply the right tool for the job. Just don't over-use it.

MarkMLl
MT+86 & Turbo Pascal v1 on CCP/M-86, multitasking with LAN & graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.
GitHub repositories: https://github.com/MarkMLl?tab=repositories

MaxCuriosus

  • Full Member
  • ***
  • Posts: 136
Re: Eliminating Goto/Label
« Reply #5 on: July 18, 2020, 02:36:13 pm »
marcov,
although you didn't use specifically a state variable, the combine use of "break" and "continue" acts as a kind of flip-flop flag or state variable.
All the other proposed solutions also use a form or another of implicit or explicit state variable. And contrary to what others seem to think your code has sufficient clarity, for me at least.
So you get the cigar.

MaxCuriosus

  • Full Member
  • ***
  • Posts: 136
Re: Eliminating Goto/Label
« Reply #6 on: July 18, 2020, 02:38:12 pm »
Warfley,
all your detailed points are well taken. I'd like to add the following:

1. Rather than use try-finally in relation to condition1, I'd prefer to use an additional procedure calling the "LoopCode" and add the "exit" code #14 in it. That
will lighten a little the main code.

2. With the exception of { Code #5 } all the other { code #x } are already reduced to 3-5 lines, one of which is a function with a return value used as a condition. So creating a new function to replace 3-5 lines doesn't add much to clarity.

3. While your procedure "DispLoop" and boolean "Reload" eliminate the explicit loops and reduce the lines of codes, that does not improve clarity w/r/t marcov's solution, in my opinion.

MaxCuriosus

  • Full Member
  • ***
  • Posts: 136
Re: Eliminating Goto/Label
« Reply #7 on: July 18, 2020, 02:40:25 pm »
440bx,
1. What's the point of using two state variables rather than just one, as Warfley has done?

2. What spaghetti code? The structure of the procedure is straigtforward, as Warfley has already summarized. Or in other words, setting aside the the initial condition and the exit code, it's simply:
 
a triage based on the value of a parameter;
the corresponding execution of code with conditions;
and the return to the beginning of the inner or outer loop.

The fact that the triage has to deal with many values of the parameter VK doesn't constitute "spaghetti" code, nor does the number of conditions used. As explained in my previous post many of those conditions are in fact the return value of a function.

MaxCuriosus

  • Full Member
  • ***
  • Posts: 136
Re: Eliminating Goto/Label
« Reply #8 on: July 18, 2020, 02:41:58 pm »
MarkMLI,
I concur, adding that "endless loops" are similar to "goto backwards".
I generally avoid the use of goto-label, but in this particular case I found it easier to use them, although the "Lexit" label can be easily removed, and will be.

Warfley

  • Hero Member
  • *****
  • Posts: 1499
Re: Eliminating Goto/Label
« Reply #9 on: July 18, 2020, 03:46:02 pm »
And contrary to what others seem to think your code has sufficient clarity, for me at least.
So you get the cigar.

As someone who without knowing anything about your code has read marcovs code before yours (when writing my post marcovs post was right below the editor so i looked there to check what your code was doing while translating it to my solution), I would suggest you, rather than using the loop based code, to stick with goto. Your goto code was understandable by just scrolling through once. With marcovs code, I had to scroll back a few times to remind myself on the loop structure used, because just encountering a break or continue doesn't tell you anything about the location you jump to without knowing the context.

The structure and code is essentially the same as the goto code, you just replaced named labels which by their name convey information about the jump, with anonymous break and continue statements, therefore having the same code with just less immediate information available.

I don't know why you want to remove the gotos, but the reason must be pretty good if you are willing to make your code less readable for that matter.
« Last Edit: July 18, 2020, 03:49:57 pm by Warfley »

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11383
  • FPC developer.
Re: Eliminating Goto/Label
« Reply #10 on: July 18, 2020, 04:05:55 pm »
marcov,
although you didn't use specifically a state variable, the combine use of "break" and "continue" acts as a kind of flip-flop flag or state variable.
All the other proposed solutions also use a form or another of implicit or explicit state variable. And contrary to what others seem to think your code has sufficient clarity, for me at least.
So you get the cigar.

Thanks, but I don't smoke. Can we make it a virtual bottle of wine ? : _)

Anyway, I didn't really start out with an idea to avoid state variables. It was more an attempt to remove some gotos but stay close to the original structure, as a first step in a transformation. I immediately recognized that most gotos were either a continue or a break.

The rest were the less elegant things are what to put at the end of both loops, not knowing if we get there or not, and the .Lexit code to run (always) on exit, solved withy try..finally. I can live with the latter as solution (though it takes it from purely procedural into object pascal territory), no problem. The former is a bit more complicated, and can't be answered. Whatever decisions you make, comment it thoroughly, since these are not standard constructs.

MarkMLl

  • Hero Member
  • *****
  • Posts: 6676
Re: Eliminating Goto/Label
« Reply #11 on: July 18, 2020, 04:10:29 pm »
Noting @Warfly's comment about readability, even though labels are obviously predeclared I think it's worthwhile trying to have the definition point (i.e. where the label is actually inserted in the code) before the corresponding goto. So in short, going backwards a few statements is OKish, going forwards rather less so.

I was at a conference once where one of the papers argued that any explicit instruction to terminate a loop (break/continue in the case of Pascal) was tantamount to a goto, hence should be "considered harmful" and avoided in the interest of clarity. But I think that in this thread all of the clear solutions involve one or more of those instructions, and that the solutions with extra functions and/or state variables are inferior.

Another observation I'd make is that the "mathematically inclined" seem to enjoy introducing transcendentals into expressions simply to avoid an "if this is <1 then do this" type of decision. They then attempt to justify the practice by claiming that introducing a sin() or cos() is actually more efficient than the pipeline stall and potential cache miss that a straightforward control transfer might cause. A little knowledge is a dangerous thing...

MarkMLl
MT+86 & Turbo Pascal v1 on CCP/M-86, multitasking with LAN & graphics in 128Kb.
Pet hate: people who boast about the size and sophistication of their computer.
GitHub repositories: https://github.com/MarkMLl?tab=repositories

440bx

  • Hero Member
  • *****
  • Posts: 3944
Re: Eliminating Goto/Label
« Reply #12 on: July 18, 2020, 04:41:05 pm »
440bx,
1. What's the point of using two state variables rather than just one, as Warfley has done?
The point is that it is much more powerful to do that way _and_, with just a little additional code, how the whole thing flows depending on those conditions can be made a lot easier to understand.  I'm not going to say that the code below is "good" because, it still leaves something to be desired but, I do believe it is a very significant improvement over your original code:
Code: Pascal  [Select][+][-]
  1. program TestNoGoto2;
  2.  
  3.   procedure LasagnaCode;
  4.   var
  5.     Condition1 : boolean = false;
  6.     Condition2 : boolean = false;
  7.     Condition3 : boolean = false;
  8.     Condition4 : boolean = false;
  9.     Condition5 : boolean = false;
  10.     Condition6 : boolean = false;
  11.  
  12.  
  13.     Reload     : boolean = false;    { pasta  layer                           }
  14.     DispLoop   : boolean = false;    { cheese layer                           }
  15.  
  16.     Scope      : integer = 1;        { used to create an inline scope         }
  17.  
  18.  
  19.     procedure ExitDispLoop;
  20.     begin
  21.       DispLoop := false;
  22.     end;
  23.  
  24.     procedure ExitDispLoopAndReloadLoop;
  25.     begin
  26.       DispLoop   := false;
  27.       ReloadLoop := false;
  28.     end;
  29.  
  30.  
  31.   begin
  32.     { no inline scopes in Pascal, "synthesize" one                            }
  33.  
  34.     for Scope := Scope to Scope do   { scope to break out of                  }
  35.     begin
  36.       if Condition1 then
  37.       begin
  38.         break;                  { empty plate : no lasagna today              }
  39.       end;
  40.  
  41.       Reload := true;           { Reload loop control variable                }
  42.       while Reload do
  43.       begin
  44.  
  45.         DispLoop := true;       { DispLoop control variable                   }
  46.         while DispLoop do
  47.         begin
  48.  
  49.           if Condition2 then
  50.           begin
  51.             ExitDispLoop;      { end DispLoop and continue Reload loop        }
  52.             break;
  53.           end;
  54.  
  55.           { NOTE: better than testing for Condition3 and nesting a "case"     }
  56.           {       statement in it, would be to test for "not condition3" and  }
  57.           {       simply exit, thus:                                          }
  58.           {                                                                   }
  59.           {       if not Condition3 then                                      }
  60.           {       begin                                                       }
  61.           {         ExitDispLoopAndReloadLoop();                              }
  62.           {                                                                   }
  63.           {         break;                                                    }
  64.           {       end;                                                        }
  65.  
  66.           if Condition3 then
  67.           begin
  68.             case Vk of
  69.               0 : begin
  70.                     ExitDispLoopAndReloadLoop();
  71.                     break;
  72.                   end;
  73.  
  74.               1 : begin
  75.                     if Condition4 then
  76.                     begin
  77.                       continue;          { start new DispLoop iteration       }
  78.                     end;
  79.  
  80.                     ExitDispLoop();
  81.                     break;               { start a new Reload iteration       }
  82.                   end;
  83.  
  84.               2 : begin
  85.                     ExitDispLoop();
  86.                     break;               { start a new Reload iteration       }
  87.                   end;
  88.  
  89.               3 : begin
  90.                     if Condition5 then
  91.                     begin
  92.                       continue;          { start new DispLoop iteration       }
  93.                     end;
  94.  
  95.                     ExitDispLoop();
  96.                     break;               { start a new Reload iteration       }
  97.                   end;
  98.  
  99.               4 : begin
  100.                     if Condition6 then
  101.                     begin
  102.                       continue;          { start new DispLoop iteration       }
  103.                     end;
  104.  
  105.                     if Condition7 then
  106.                     begin
  107.                       ExitDispLoop();
  108.                       break              { start a new Reload iteration       }
  109.                     end;
  110.                   end;
  111.  
  112.               5 : begin
  113.                     continue;            { start new DispLoop iteration       }
  114.                   end;
  115.  
  116.               { it would be a good idea to have an "otherwise" for safety     }
  117.  
  118.               otherwise
  119.               begin
  120.                 { whatever should be done in this case                        }
  121.  
  122.                 ;          { do nothing - meditate over infinite lasagna      }
  123.               end;
  124.             end; { case }
  125.  
  126.           end; { Condition3 }
  127.  
  128.           { should any instructions be executed when the DispLoop is broken   }
  129.           { out of but ReloadLoop will be executed again ?                    }
  130.  
  131.           { if ReloadLoop true/false then do whatever                         }
  132.  
  133.         end; { DispLoop }
  134.  
  135.       end; { Reload loop }
  136.  
  137.     end; { for scope }
  138.  
  139.     { do whatever here - surgeon general warning                              }
  140.  
  141.     writeln('Abuse of lasagna and spaghetti are not healthy');
  142.   end;
  143.  
  144.  
  145. begin
  146.   LasagnaCode();
  147. end.
  148.  
That way, a programmer that reads that can much more easily visualize what happens and when.  Something you don't get with "goto"(s).

2. What spaghetti code? The structure of the procedure is straigtforward, as Warfley has already summarized. Or in other words, setting aside the the initial condition and the exit code, it's simply:
Spaghetti code is any code that causes the programmer's attention to be jumping around in a piece of code.  "goto"s are not the only way to make execution jump around.  The answer to your question is: the code you posted is spaghetti code and the one I posted is dangerously close to spaghetti code (it's lasagna code, strands of cheese spaghetti between layers of pasta.)


The fact that the triage has to deal with many values of the parameter VK doesn't constitute "spaghetti" code, nor does the number of conditions used. As explained in my previous post many of those conditions are in fact the return value of a function.
As stated above, what makes it spaghetti code is that the programmer's attention has to jump all over the place to figure out how that code flows.  That's bad/niente/no good/trump.
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

egsuh

  • Hero Member
  • *****
  • Posts: 1273
Re: Eliminating Goto/Label
« Reply #13 on: July 19, 2020, 06:11:58 am »
Why not just write procedures  Reload, DispLoop and Lexit? You may define function/procedure within function/procedure.

process_1

  • Guest
Re: Eliminating Goto/Label
« Reply #14 on: July 19, 2020, 09:02:55 am »
@MaxCuriosus

Using goto in procedural language as it is Pascal is a mess, but also using too many break and continue make code as well quite messy.

I don't have much time right now to look in your original code in details, but this kind of code can be much clear, i.e. easier to control with additional Boolean variables instead too use too many break and continue loop control.

The tipical code of this type is when parsing text.

 

TinyPortal © 2005-2018