Lesson Learned!

[J-G]

Unusually, I'm not looking for a solution to a problem (that is normally why I start a thread!) I have solved the issue but thought that it, the reason and the solution might be of interest.

I've been using my accouning package, originally written under Lazarus 1.6, for some years with very few problems. Earlier this year I had a problem with a new project which resulted in my updating to Lazarus 2.2.0 so naturally 'tweaks' to my existing programs are now also done under this.

Most of my invoices are single line items, three lines are unusual so I had allowed for a max of 5 which I think I have used less than 10 times in 30 years. Today I had to raise an invoice which has 5 lines and is the first time since using Laz 2.2.

Everything was 'normal' until I came to [Print] the invoice, whereupon I had a "201 - Range Check Error"  !!!    - - - -   when I ran the project with de-bugging it seemed most peculiar to find the error was fired by a simple [ inc(c) ] instruction. The particular section of code was :

Code: Pascal  [Select][+][-]

1.    
2. While (Invoice.item[c].Desc > '') and (c < 6) Do
3.    begin
4. [...]
5.     inc(c)
6.   end;
7.  

Under Laz 1.6 this was perfectly acceptable but it seems that Laz 2.2 is not happy at all 

Compiling with 'Range Checking OFF' did not solve it.

The problem is of course that [Invoice.items] is declared as an array of 5 elements so naturally [c] can never be 6 - though why it fired the error at 'inc(c)' rather than 'While (Invoice.item[c].Desc ...' is still a mystery to me.

My solution was to introduce a new Boolean ('Done') and modify the proc thus :

Code: Pascal  [Select][+][-]

1.    
2. Done := false;
3. While (Invoice.item[c].Desc > '') and (not Done) Do
4.    begin
5. [...]
6.     if c < 5 then
7.        inc(c)
8.      else
9.        Done := True;
10.    end;
11.  

I hope this explanation may be of some small benefit.

[mikerabat]

hm... interesting...
how is c declared? If it's an integer than this simply may not happen and my guts tell me then this is not the problem
of incrementing c but somthing like compiler shortcut...

Also.. if c is an integer then you should change the boolean evaluation to

Code: Pascal  [Select][+][-]

1. while (c < Length(Invoice.item)) and (Invoice.item[c].Desc > '') Do
2.  

Boolean evaluations are normally evaluated from left to right and stops early (except {$B +} is defined in the unit) so honestly
the range check error should have been there in earlier versions too!

[MarkMLl]

You'd been quiet, good to hear you're OK.

This is, in fact, technically wrong:

Code: Pascal  [Select][+][-]

1.    
2. While (Invoice.item[c].Desc > '') and (c < 6) Do
3.  

Irrespective of whether the compiler shortcircuits Boolean expressions, you're always checking the 6th item irrespective of whether it exists. Try

Code: Pascal  [Select][+][-]

1.    
2. While (c < 6) and (Invoice.item[c].Desc > '') Do
3.  

...or even break it into two conditionals, if you're wary of the shortcircuiting.

As I mentioned a few days ago, I've seen compilers which actually documented the  if  statement as though  and, not, or were part of the syntax, rather than shortcircuiting as soon as the result was known.

MarkMLl

[Josh]

Hi

would a <= work better than <
ie

Code: Pascal  [Select][+][-]

1. const max_inv_items=5;
2. ..
3. procedure do_some_stuff;
4. var c:integer;
5. begin
6.   ....
7.   c:=1; // assuming array defined as[1..5] or easier [1..max_inv_items]
8.   While ( (Invoice.item[c].Desc <> '') and  (c <=max_inv_items) ) Do
9.    begin
10. [...]
11.     inc(c)
12.   end;
13. end;
14.  

or a simple for loop

Code: Pascal  [Select][+][-]

1. const max_inv_items=5;
2. ..
3. procedure do_some_stuff;
4. var c:integer;
5. begin
6.   for c:=1 to max_inv_items do
7.   begin
8.     if Invoice.item[c].Desc <>'' then
9.     begin
10.        .....
11.     end;
12.   end;
13. end;

note the difference in operation though, imagine array ispopilated fully with text, both produce the same, if you set arrray elemebt 3 to '';then while loop stops at 3 and does not continue to 4 and 5, for loop will iterate all elements.

[Thaddy]

No, because inc() will still overflow using <=. Must be < in this code example because of the place the inc().
Anyway I do not get it that older versions did NOT throw a range check error. (That is not true, period)
I can not believe that. Must have been a setting.

And by now such code should be replaced by for in do which solves the problem either way.

[J-G]

With the best will in the world I still forget to include all the information that I ought 

[ c ] is declared as 'Byte'  - it will only ever be 1 through 5.

I agree that I would have expected the Range Check error to also be in 1.6 - though when I wrote the package I don't think I knew about turning Range Checking ON.

Your suggestion looks to be more efficient than my addition of 'Done' and I hadn't appreciated the significance of the 'Left to Right' evaluation so I'll see whether it does the job when I next have to 'tweak'.

[MarkMLl]

Your suggestion looks to be more efficient than my addition of 'Done' and I hadn't appreciated the significance of the 'Left to Right' evaluation so I'll see whether it does the job when I next have to 'tweak'.

I'm wary of it, for the reason I alluded to. I don't know whether there really are compilers which treat  and  etc. as a syntactic keyword rather than an operator with the short-circuit trait, or if the explanation I read (which was exceptionally heavy going) was entirely a documentation quirk.

MarkMLl

[J-G]

You'd been quiet, good to hear you're OK.

Thanks Mark - yes I've been back in my workshop making Christmas presents rather than writing solutions to interesting (though mostly fatuous) questions   

This is, in fact, technically wrong:

Code: Pascal  [Select][+][-]

1.    
2. While (Invoice.item[c].Desc > '') and (c < 6) Do
3.  

Irrespective of whether the compiler shortcircuits Boolean expressions, you're always checking the 6th item irrespective of whether it exists. Try

I've just seen your further responce so I'll also bear that in mind when I look at the code again - must do some more 'accounting'  -  I've just had notification that the Invoice that caused the hiccup has been paid!!  Result 

[J-G]

Thanks @Josh & @Thaddy  for your input - this is just an acknowledgement that I have seen and read your response  -  It does not mean that I have understood the implications!! 

I will be looking at the code again though.

[Josh]

just tested

if you change the offending line to, ie swap them around so

Code: Pascal  [Select][+][-]

1. While ( (Invoice.item[c].Desc <> '') and  (c <=max_inv_items) ) Do

becomes

Code: Pascal  [Select][+][-]

1. While ( (c <=max_inv_items) and (Invoice.item[c].Desc <> '') ) Do

shouldsolve it