Formatting in FreePascal

[PascalDragon]

I waited a few days to see if anyone who uses that "style" would notice that the above is _incorrect_.

Those that use that style simply don't care that you think it's “incorrect”.

[440bx]

I waited a few days to see if anyone who uses that "style" would notice that the above is _incorrect_.

Those that use that style simply don't care that you think it's “incorrect”.

Various replies in this thread had already made that quite evident.

I understand that those who use that style don't care whether or not I think it is correct but, they _might_ care that the incorrectly formatted code lead to your introducing a bug.  Specifically, the original snippet of code is:

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

1. with TBarSeries(Result) do
2. begin
3.   <snip>
4.  
5.   if APlotType = ptBars then
6.   begin
7.     AxisIndexX := AChart.LeftAxis.Index;
8.     AxisIndexY := AChart.BottomAxis.Index;
9.   end
10.   else
11.   begin
12.     AxisIndexX := AChart.BottomAxis.Index;
13.     AxisIndexY := AChart.LeftAxis.Index;
14.   end;
15. end;

which you formatted incorrectly like this:

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

1. with TBarSeries(Result) do begin
2.   <snip>
3.   if APlotType = ptBars then begin
4.     AxisIndexX := AChart.LeftAxis.Index;
5.     AxisIndexY := AChart.BottomAxis.Index;
6.   end else begin
7.     AxisIndexX := AChart.BottomAxis.Index;
8.     AxisIndexY := AChart.LeftAxis.Index;
9.   end;
10. end;

to drive the point home that formatting is incorrect, I asked this:

And if you need to add another "if" after that "begin" where is the "end" of that "if" going to be ?... is it going to be a third "end" to the starting if ?

To which you replied with this

Correct:

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

1. with TBarSeries(Result) do begin
2.   <snip>
3.   if APlotType = ptBars then begin
4.     AxisIndexX := AChart.LeftAxis.Index;
5.     AxisIndexY := AChart.BottomAxis.Index;
6.   end else if APlotType = ptWhatEver then begin
7.     // whatever
8.   end else begin
9.     AxisIndexX := AChart.BottomAxis.Index;
10.     AxisIndexY := AChart.LeftAxis.Index;
11.   end;
12. end;
13.  

The problem is, adding an "if" statement should have produced this:

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

1. with TBarSeries(Result) do
2. begin
3.   <snip>
4.  
5.   if APlotType = ptBars then
6.   begin
7.     AxisIndexX := AChart.LeftAxis.Index;
8.     AxisIndexY := AChart.BottomAxis.Index;
9.   end
10.   else
11.   begin
12.     if APlotType = ptWhatEver then <whatever>;   // added if statement
13.  
14.     AxisIndexX := AChart.BottomAxis.Index;
15.     AxisIndexY := AChart.LeftAxis.Index;
16.   end;
17. end;

instead, your adding the "if" statement, produced this (formatted correctly to make the error visible):

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

1. with TBarSeries(Result) do begin
2.   <snip>
3.   if APlotType = ptBars then
4.   begin
5.     AxisIndexX := AChart.LeftAxis.Index;
6.     AxisIndexY := AChart.BottomAxis.Index;
7.   end
8.   else
9.     if APlotType = ptWhatEver then
10.     begin
11.       // whatever
12.     end
13.     else
14.     begin
15.        AxisIndexX := AChart.BottomAxis.Index;
16.        AxisIndexY := AChart.LeftAxis.Index;
17.     end;
18. end;
19.  

Now the conditions that control when the statements

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

1.        AxisIndexX := AChart.BottomAxis.Index;
2.        AxisIndexY := AChart.LeftAxis.Index;

get executed are no longer what they should be.

However, I have no doubt that's what you intended.  Changing the conditions that control the execution of the statements is included in the formatting "style" too.

[kupferstecher]

Just compile it. Its the same.

Edit: And that shows the beauty of that formatting, no nesting. Just a simple if..elseif..else..end structure.

[440bx]

Its the same.

No, not the same.   The statements

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

1.  AxisIndexX := AChart.BottomAxis.Index;
2.  AxisIndexY := AChart.LeftAxis.Index;

are no longer executed based on the same conditions.

Edit: And that shows the beauty of that formatting, no nesting. Just a simple if..elseif..else..end structure.

When introducing bugs in code is considered "beautiful" that formatting will be "beautiful" too.

[kupferstecher]

Yes, you're right, it's not the same. But your example is useless. Of course you can add a new level of ifs, but thats not a matter of formatting. This was about inserting an "else if" and than the code is correct.

[RAW]

I like this ...

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

1. With TBarSeries(Result)
2. Do
3.  Begin
4.    <snip>
5.  
6.    If APlotType = ptBars
7.    Then
8.     Begin
9.       AxisIndexX:= AChart.LeftAxis.Index;
10.       AxisIndexY:= AChart.BottomAxis.Index;
11.     End
12.    Else
13.     Begin
14.       If APlotType = ptWhatEver
15.       Then <whatever>;
16.  
17.       AxisIndexX:= AChart.BottomAxis.Index;
18.       AxisIndexY:= AChart.LeftAxis.Index;
19.     End;
20.  End;

Yeah, I couldn't resist ... 

[440bx]

Yes, you're right, it's not the same.

We're making progress.  That's good.

But your example is useless.

It wasn't an example. It was a request that an "if" statement be added.

Of course you can add a new level of ifs, but thats not a matter of formatting. This was about inserting an "else if" and than the code is correct.

The formatting is the main cause why when that "if" was added a bug was introduced.

When the formatting is correct, it's rather unlikely for a programmer to make that mistake.


ETA:

Yeah, I couldn't resist ... 

That's better than upside-down

[PascalDragon]

Now the conditions that control when the statements

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

1.        AxisIndexX := AChart.BottomAxis.Index;
2.        AxisIndexY := AChart.LeftAxis.Index;

get executed are no longer what they should be.

However, I have no doubt that's what you intended.  Changing the conditions that control the execution of the statements is included in the formatting "style" too.

You are aware that I merely added the if-statement to illustrate the formatting and did not care at all about the functionality of the code?

[wp]

All the following variants are "correct" for me:

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

1. with TBarSeries(Result) do begin
2.   if APlotType = ptBars then begin
3.     AxisIndexX := AChart.LeftAxis.Index;
4.     AxisIndexY := AChart.BottomAxis.Index;
5.   end else
6.   if APlotType = ptWhatEver then begin
7.     AxisIndexX := something;
8.     AxisIndexY := something_else;
9.   end else begin
10.     AxisIndexX := AChart.BottomAxis.Index;
11.     AxisIndexY := AChart.LeftAxis.Index;
12.   end;
13. end;
14. ...
15. with TBarSeries(Result) do
16. begin
17.   if APlotType = ptBars then
18.   begin
19.     AxisIndexX := AChart.LeftAxis.Index;
20.     AxisIndexY := AChart.BottomAxis.Index;
21.   end
22.   else
23.   if APlotType = ptWhatEver then
24.   begin
25.     AxisIndexX := something;
26.     AxisIndexY := something_else;
27.   end
28.   else
29.   begin
30.     AxisIndexX := AChart.BottomAxis.Index;
31.     AxisIndexY := AChart.LeftAxis.Index;
32.   end;
33. end;
34. ...
35. with TBarSeries(Result) do begin
36.   if APlotType = ptBars then
37.   begin
38.     AxisIndexX := AChart.LeftAxis.Index;
39.     AxisIndexY := AChart.BottomAxis.Index;
40.   end
41.   else
42.     if APlotType = ptWhatEver then
43.     begin
44.       AxisIndexX := something;
45.       AxisIndexY := something_else;
46.     end
47.     else
48.     begin
49.        AxisIndexX := AChart.BottomAxis.Index;
50.        AxisIndexY := AChart.LeftAxis.Index;
51.     end;
52. end;

Personally I prefer the first one because it requires the least lines on the screen. What I absolutely do not like is missing indentation and I put its importance far above the question whether the "begin" after an "if" should be on the same line or not:

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

1. // DO NOT DO THIS!
2. with TBarSeries(Result) do begin
3. if APlotType = ptBars then begin
4. AxisIndexX := AChart.LeftAxis.Index;
5. AxisIndexY := AChart.BottomAxis.Index;
6. end else
7. if APlotType = ptWhatEver then begin
8. AxisIndexX := something;
9. AxisIndexY := something_else;
10. end else begin
11. AxisIndexX := AChart.BottomAxis.Index;
12. AxisIndexY := AChart.LeftAxis.Index;
13. end;
14. end;

[howardpc]

Personally, I much prefer this construction, which I find far easier to read and comprehend at a glance.
I believe in some cases it may also be a tad faster, but for me that is unimportant compared to readability and ease of maintenance.

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

1.   with TBarSeries(Result) do
2.     case APlotType of
3.       ptBars:
4.         begin
5.           AxisIndexX := AChart.LeftAxis.Index;
6.           AxisIndexY := AChart.BottomAxis.Index;
7.         end;
8.       ptWhatever:
9.         begin
10.           AxisIndexX := something;
11.           AxisIndexY := something_else;
12.         end;
13.       else
14.           AxisIndexX := AChart.BottomAxis.Index;
15.           AxisIndexY := AChart.LeftAxis.Index;
16.     end;
17.   end;

[Thaddy]

I totally agree with you. Except the indentation of the else clause  and that bug it contains   (no begin end)

IMHO you should not use whitespace at all and make it readable with ptop afterwards...

[fred]

@howardpc: Line 17 end at the same level as with but no begin 

[440bx]

You are aware that I merely added the if-statement to illustrate the formatting and did not care at all about the functionality of the code?

Of course, why would anyone care about the functionality, it's so much more important to save a line or two.

[ASerge]

@howardpc: Line 17 end at the same level as with but no begin 

Yes, yes when "begin" is on a separate line, the probability of such an error is less, because only "case" and "begin" end on the "end". In other cases, this is a mistake. And if you write "begin" anywhere - you have to carefully read each complex structure.

[howardpc]

I totally agree with you. Except the indentation of the else clause  and that bug it contains   (no begin end)

An else clause within a case construct does not require a begin/end since the "else" and the case's "end" are sufficient.
You can add one if you feel the need to write one and make each section of the case construct symmetrical.
But that is a matter of aesthetics, not of correct or incorrect syntax.
I think Pascal is verbose enough. No need to add unnecessary begin/end pairs.
Describing this as a bug is wrong.
The compiler developers don't consider it a bug.
"with" and "case" are both terminated by "end" correctly.
Neither needs a corresponding "begin".

Not every "end" has a matching "begin".