Puzzling small piece of code - exception block essentially used as goto

Puzzling small piece of code - exception block essentially used as goto

[Anonymous Coward]

This is an excerpt from some code I recently encountered (it may give
some of you a headache):

  Still_Looping := True;

  Delta_Beyond_Max_Minutes:
    while Still_Looping loop

      Handle_Max_Minute_Error:
        begin

          Hour_Delta    := New_Hour - Hour;
          Still_Looping := False;
          Hour          := New_Hour;

        exception
          when Constraint_Error =>

            if New_Hour > Hour then

              Hour_Delta := Max_Hour_Delta

            else

              Hour_Delta := Relative_Time - Max_Hour_Delta;

            end if;

            Hour := Hour + Hour_Delta;

        end Handle_Max_Minute_Error;
    end loop Delta_Beyond_Max_Minutes;

Tests show that this code actually loops a few times when run in a
typical state.  What I would like clarification on is when the
exception block ends, where is control returned to?

Also, anyone up for the challenge of rewriting this without the
exception handler so the rest of us humans can understand what it's
doing?

Re: Puzzling small piece of code - exception block essentially used as goto

[Jeffrey R. Carter]

Anonymous Coward wrote:
> 
> Tests show that this code actually loops a few times when run in a
> typical state.  What I would like clarification on is when the
> exception block ends, where is control returned to?

Control continues from the end of the block statement.

> Also, anyone up for the challenge of rewriting this without the
> exception handler so the rest of us humans can understand what it's
> doing?

Presumably the difficulty is evaluating the expression New_Hour - Hour. 
It may be impossible to determine which of the 3 calculations of 
Hour_Delta to use without evaluating the expression and seeing if it 
raises Constraint_Error. Or perhaps determining which of the 3 
calculations of Hour_Delta to use is more complex than this code. 
Without knowing the details of the types of the variables used, I can't say.

-- 
Jeff Carter
"Propose to an Englishman any principle, or any instrument, however
admirable, and you will observe that the whole effort of the English
mind is directed to find a difficulty, a defect, or an impossibility
in it. If you speak to him of a machine for peeling a potato, he will
pronounce it impossible: if you peel a potato with it before his eyes,
he will declare it useless, because it will not slice a pineapple."
Charles Babbage
92

Re: Puzzling small piece of code - exception block essentially used as goto

[tmoran]

> What I would like clarification on is when the
> exception block ends, where is control returned to?
    Control isn't "returned" anywhere.  It flows to the next statement
after the block.  In this case the next statement is:
>   end loop Delta_Beyond_Max_Minutes;

> Also, anyone up for the challenge of rewriting this without the
> exception handler so the rest of us humans can understand what it's
> doing?
Assuming that Hour is the only output and Hour_Delta and
Still_Looping are not used anywhere else,

Set Hour to (approximately) New_Hour.  Specifically, if New_Hour
is sufficiently close to Hour (ie, New_Hour - Hour  fits in the
legal range of Hour_Delta), then set Hour := New_Hour.
Otherwise, step Hour forward by Max_Hour_Delta, or backward
by (Max_Hour_Delta - Relative_Time), as many times as necessary to
make  New_Hour - Hour  fit in that range.

It would be helpful to know the range of Hour_Delta and to have some idea
what Max_Hour_Delta and Relative_Time might be, to figure out just what
the programmer was trying to accomplish.

Re: Puzzling small piece of code - exception block essentially used as goto

[Adam Beneschan]

On Jun 1, 10:01 pm, Anonymous Coward <anonym...@coward.org> wrote:
> This is an excerpt from some code I recently encountered (it may give
> some of you a headache):
>
>   Still_Looping := True;
>
>   Delta_Beyond_Max_Minutes:
>     while Still_Looping loop
>
>       Handle_Max_Minute_Error:
>         begin
>
>           Hour_Delta    := New_Hour - Hour;
>           Still_Looping := False;
>           Hour          := New_Hour;
>
>         exception
>           when Constraint_Error =>
>
>             if New_Hour > Hour then
>
>               Hour_Delta := Max_Hour_Delta
>
>             else
>
>               Hour_Delta := Relative_Time - Max_Hour_Delta;
>
>             end if;
>
>             Hour := Hour + Hour_Delta;
>
>         end Handle_Max_Minute_Error;
>     end loop Delta_Beyond_Max_Minutes;
>
> Tests show that this code actually loops a few times when run in a
> typical state.  What I would like clarification on is when the
> exception block ends, where is control returned to?

It goes to the point after the block it's part of, i.e.
Handle_Max_Minute_Error.  At that point, it hits the "end loop" and
loops back, if Still_Looping is still true.

It looks like the code is expecting that a Constraint_Error might be
raised on

   Hour_Delta := New_Hour - Hour;

Since I don't see the declarations of any of those variables, I don't
know whether it's expecting a Constraint_Error on the subtraction
operation, or on the assignment of the final result to Hour_Delta (if
Hour_Delta is a subrange).  But the purpose of the code seems to be
that: if the above subtraction succeeds, we're done; if not, it
adjusts some values and tries again.


> Also, anyone up for the challenge of rewriting this without the
> exception handler so the rest of us humans can understand what it's
> doing?

As noted above, this is really impossible without knowing how those
three key variables are declared.

However, it's possible to write something more readable even with an
exception handler.  Here's how I might do it:

Delta_Beyond_Max_Minutes:
   loop
      begin
         Hour_Delta := New_Hour - Hour;
         Subtraction_Succeeds := True;
      exception
         when Constraint_Error =>
            Subtraction_Succeeds := False;
      end;
      if Subtraction_Succeeds then
         Hour := New_Hour;
         exit;
      else
         if New_Hour > Hour then
            Hour_Delta := Max_Hour_Delta
         else
            Hour_Delta := Relative_Time - Max_Hour_Delta;
         end if;
         Hour := Hour + Hour_Delta;
      end if;
   end loop Delta_Beyond_Max_Minutes;

Is this easier to understand?

(I'm assuming that "Hour := New_Hour;" won't raise an exception.)

                  -- Adam

Re: Puzzling small piece of code - exception block essentially used as goto

[Anonymous Coward]

On 2007-06-02, Jeffrey R. Carter <spam.jrcarter.not@acm.nospam.org> wrote:
>
> Without knowing the details of the types of the variables used, I
> can't say.

Here are the ranges for the variables (all are integers):

Hour_Delta: -9999*60..9999*60
New_Hour:          0..2^32
Hour:              0..2^32

Relative_Time : constant := 0;

Re: Puzzling small piece of code - exception block essentially used as goto

[tmoran]

> > Without knowing the details of the types of the variables used, I
> > can't say.
>
> Here are the ranges for the variables (all are integers):
>
> Hour_Delta: -9999*60..9999*60
> New_Hour:          0..2^32
> Hour:              0..2^32
>
> Relative_Time : constant := 0;
  And what is the value of Max_Hour_Delta?

Another way to look at that code is to observe that the only way out of
the loop is to not raise Constraint_Error, ie, New_Hour is within
+-9999*60 of Hour (that *60 rather suggests the units are minutes, not
hours as one might assume fron the names).  If that's not true at first,
the code adjusts Hour by steps of Max_Delta_Hour, hopefully in the
direction of New_Hour.  If it doesn't overshoot, Hour will eventually be
in range, at which time it will be set to the value of New_Hour.  So the
code essentially says:

if Max_Delta_Hour < 0 then
  burn some CPU time;      -- move Hour ever farther from New_Hour
  raise Constraint_Error;  -- till it hits a limit
elsif Max_Delta_Hour = 0 then
  sit forever in infinite loop, never changing Hour;
elsif Max_Delta_Hour in > 2 * 9999*60
                          + 1
                          + ( abs(New_Hour-Hour) mod (9999*60+1) ) then
  oscillate forever between Hour too positive and Hour too negative;
else
  burn some CPU time;
end if;
Hour := New_Hour;
(if I did my algebra correctly).

So replacing the whole thing by "Hour := New_Hour;" probably does
what the original programmer wanted.

Re: Puzzling small piece of code - exception block essentially used as goto

[tmoran]

Of course that should be
if New_Hour - Hour in -9999*60 .. 9999*60 then
  null;  -- no funny stuff
elsif Max_Delta_Hour < 0 then
  etc.

Re: Puzzling small piece of code - exception block essentially used as goto

[anon]

Looks like a HOMEWORK problem to me:
And I do not help students cheat!

In <vQ68i.99$UD4.49@trndny07>, Anonymous Coward <anonymous@coward.org> writes:
>This is an excerpt from some code I recently encountered (it may give
>some of you a headache):
>
>  Still_Looping := True;
>
>  Delta_Beyond_Max_Minutes:
>    while Still_Looping loop
>
>      Handle_Max_Minute_Error:
>        begin
>
>          Hour_Delta    := New_Hour - Hour;
>          Still_Looping := False;
>          Hour          := New_Hour;
>
>        exception
>          when Constraint_Error =>
>
>            if New_Hour > Hour then
>
>              Hour_Delta := Max_Hour_Delta
>
>            else
>
>              Hour_Delta := Relative_Time - Max_Hour_Delta;
>
>            end if;
>
>            Hour := Hour + Hour_Delta;
>
>        end Handle_Max_Minute_Error;
>    end loop Delta_Beyond_Max_Minutes;
>
>Tests show that this code actually loops a few times when run in a
>typical state.  What I would like clarification on is when the
>exception block ends, where is control returned to?
>
>Also, anyone up for the challenge of rewriting this without the
>exception handler so the rest of us humans can understand what it's
>doing?

Re: Puzzling small piece of code - exception block essentially used as goto

[Markus E Leypold]

> Looks like a HOMEWORK problem to me:
> And I do not help students cheat!

"anon" <anon@anon.org> won't help "Anonymous Coward"
<anonymous@coward.org>. How funny :-)

- M

Re: Puzzling small piece of code - exception block essentially used as goto

[Anonymous Coward]

On 2007-06-05, anon <anon@anon.org> wrote:
>
> Looks like a HOMEWORK problem to me:
> And I do not help students cheat!

No, actually that is production code.  And what's more interesting, it
even passed a formal peer review.  It was raised as an issue at the
meeting, but in the end the developer rejected the feedback.

Re: Puzzling small piece of code - exception block essentially used as goto

[Markus E Leypold]

> On 2007-06-05, anon <anon@anon.org> wrote:
>>
>> Looks like a HOMEWORK problem to me:
>> And I do not help students cheat!
>
> No, actually that is production code.  And what's more interesting, it
> even passed a formal peer review.  It was raised as an issue at the
> meeting, but in the end the developer rejected the feedback.

I can understand why you're posting as "Anonymous Coward" :-).

Regards -- Markus

Re: Puzzling small piece of code - exception block essentially used as goto

[Anonymous Coward]

On 2007-06-06, Markus E Leypold <development-2006-8ecbb5cc8aREMOVETHIS@ANDTHATm-e-leypold.de> wrote:
>
> I can understand why you're posting as "Anonymous Coward" :-).

Are you implying that the quality of the code is reasonable, and the
one opposed to it should be a coward for objecting to it?  Or are you
saying the code quality is poor, and the author should be a coward?

Re: Puzzling small piece of code - exception block essentially used as goto

[Markus E Leypold]

> On 2007-06-06, Markus E Leypold <development-2006-8ecbb5cc8aREMOVETHIS@ANDTHATm-e-leypold.de> wrote:
>>
>> I can understand why you're posting as "Anonymous Coward" :-).
>
> Are you implying that the quality of the code is reasonable, and the
> one opposed to it should be a coward for objecting to it?  Or are you
> saying the code quality is poor, and the author should be a coward?

No. I'm implying that you're probably in this instance are swimming
against the main current of a corporate policy and therefore wouldn't
want to have this episode/request associated with your name. Apart
from implying that a colleague wrote bad code and refuses insight in a
code review (bad attitude for a professional). However true that might
be -- saying it in public in a way that your company might be
identified, perhaps even the colleague, that might be taken ill by
your employer or your colleague.

Or -- why ARE you posting as AC?

:-)

Regards -- Markus