Well, each developer tends to have his own set of rules be that for code formatting, identifier naming or whatever. And each developer, when looking at someone else's code tends to go like: "wow: what was this guy thinking?"...
Here's a piece of auto-generated code from Add-In Express:
procedure ActivateEvent(Sender: TObject);
procedure ClickEvent(Sender: TObject);
procedure CreateEvent(Sender: TObject);
procedure DblClickEvent(Sender: TObject);
procedure DeactivateEvent(Sender: TObject);
procedure DestroyEvent(Sender: TObject);
procedure KeyPressEvent(Sender: TObject; var Key: Char);
procedure PaintEvent(Sender: TObject);
I find this very annoying and hard to read/browse! So, even though I know it serves no useful purpose other than please my eyesight, I turn that into something like this:
procedure ActivateEvent ( Sender: TObject );
procedure ClickEvent ( Sender: TObject );
procedure CreateEvent ( Sender: TObject );
procedure DblClickEvent ( Sender: TObject );
procedure DeactivateEvent( Sender: TObject );
procedure DestroyEvent ( Sender: TObject );
procedure KeyPressEvent ( Sender: TObject; var Key: Char);
procedure PaintEvent ( Sender: TObject );
Now, isn't this 2nd version much easier on the eyes? Surely it compiles the same as the 1st one no faster nor slower no more and no less bytes used, but for us, humans, it's way easier to look at!
Heck, I even do this on forms after they are stable enough and I know I won't be adding too much stuff/events to it! :) And if I do add, I'll re-sort and re-format those lines! :)
There are a ton more of things I change for cosmetic reasons, such as placing begin/end on pretty much every single if, even if it really wasn't needed (there are exceptions mainly when the one-liner if's are used as a sort of case construct where keeping them one-lined does help readability).
Again, a small sample:
if FPropertyPageSite = nil then
if Assigned(ActiveFormControl) then
if Assigned(ActiveFormControl.ClientSite) then
ActiveFormControl.ClientSite.QueryInterface( IID_PropertyPageSite, FPropertyPageSite );
And, after reformatting:
if FPropertyPageSite = nil then begin
if Assigned(ActiveFormControl) then begin
if Assigned(ActiveFormControl.ClientSite) then begin
ActiveFormControl.ClientSite.QueryInterface( IID_PropertyPageSite, FPropertyPageSite );
end;
end;
end;
Now, you may think that there is not much difference in here, but there is! Especially if some have begin/end and others don't and depending on what code follows. Plus, always using begin/end means you don't need to look too hard on the code as to where to place the begin/end pair when instead of a single line inside a multiple if you need to have two lines of code: all the code layout is perfectly done and you just add that extra line where you want it without much trouble...
There are many more things that I change, even in code that is not my own as long as I'm the only one using it and as long as I need to change/edit/maintain it, but I thought of these as I was making some changes to this code and figured I'd take a few mins break and blog about it! :)
Also, you can take advantage of Delphi's built-in begin/end helper: just type begin at the end of a line, place your cursor to the end of the last line of the code block to be enclosed and press enter: et voilá, instant end properly lined up, courtesy of Dephi's IDE... :)
[EDIT: I knew I was being lazy with the begin/end/if example so here's a better one...]
// Messy version
begin
Result := (AControl.Handle = Focused);
if not Result then
for i := 0 to AControl.ControlCount - 1 do
if AControl.Controls[i] is TWinControl then
if TWinControl(AControl.Controls[i]).Handle = Focused then
Result := True;
else
if TWinControl(AControl.Controls[i]).ControlCount > 0 then
Result := SearchForHWND(TWinControl(AControl.Controls[i]), Focused);
end;
// Cleaned up version
begin
Result := (AControl.Handle = Focused);
if not Result then begin
for i := 0 to AControl.ControlCount - 1 do begin
if AControl.Controls[i] is TWinControl then begin
if TWinControl(AControl.Controls[i]).Handle = Focused then begin
Result := True;
end else begin
if TWinControl(AControl.Controls[i]).ControlCount > 0 then begin
Result := SearchForHWND(TWinControl(AControl.Controls[i]), Focused);
end;
end;
end;
end;
end;
end;
Of the two pieces of identical code, which one do you think is easier to add something to? Say an else to one of the if's or some other instruction in any of the "branches"... And note that, the indentation on the messy one is not that messy and it helps a bit, but you sure can follow the cleaned up version better when trying to see all possible outcomes from that function, no? It certainly works much better for me at least! :)
16 comments:
The begin/end in your if-if-if example is only necessary because you put the "begin" in the same line where the "then" is. If you put it in an extra line you can easily see where the "if" ends.
without complete booelan evaluation on (off by default) you could clean it up like this:
if (FPropertyPageSite = nil)
and
Assigned(ActiveFormControl) and Assigned(ActiveFormControl.ClientSite) then begin
ActiveFormControl.ClientSite.QueryInterface( IID_PropertyPageSite, FPropertyPageSite );
end;
Obviousely not so nicely formattet in the comment, but ... :-)
Sorry but I don't understand what you mean, Andreas: it's irrelevant if I put the begin in the same or in a new line as I will always need an end to match...
I know this was not the best example, it was lazyness of me using a simpler case I was editing at the time! :)
I'll try and update with a decent example!
This code would look like this in my coding style. So I would prefere the first one that has no begin/end. But I would place a begin/end at strategical good positions.
begin
__Result := (AControl.Handle = Focused);
__if not Result then
__begin
____for i := 0 to AControl.ControlCount - 1 do
____begin
______if AControl.Controls[i] is TWinControl then
______begin
________if TWinControl(AControl.Controls[i]).Handle = Focused then
________begin
__________Result := True;
________end
________else
________begin
__________if TWinControl(AControl.Controls[i]).ControlCount > 0 then
__________begin
____________Result := SearchForHWND(TWinControl(AControl.Controls[i]), Focused);
__________end;
________end;
______end;
____end;
__end;
end;
But it is all a matter of how you are used to the coding style. As a JEDI Developer I'm more used to the JEDI Coding style (even if I break it in some points).
When I started writing TurboPascal "applications" I used the "if .. then begin CRFL end" as I was used to the "if ... then CRLF endif" from TurboBASIC and my eyes found the matching "if/endif" very fast. But years later (must have been at the timeframe of Delphi 3), I started looking into the VCL source codes and adapted Borland's coding style as good as I wanted it. Since then the code that Delphi autogenerates fits perfectly in my own code and I can read the RTL/VCL code very fast.
Some years ago I noticed following structure in someone's code:
_if some_condition
__then begin
_______end;
It is very similar to what Andreas is using, but I put 'then' in the next line, adding some indentation.
I have found it very helpfull to read the code , so I use it every day even with other things like:
_with some_object
__do begin
_____end;
My 0.02$
Marcin
Yep, Andreas, it's a question of what each one likes most! :)
Perhaps my using of GFABasic way way back in the early 80s has something to do with the fact I like more that the begin is on the end of the previous line: since I always use it, I don't need a "visual reminder" that there is a begin on the next line. And since I couple that with a proper indentation I find it works fine for me.
Of course, I have no doubt there will be those that like it even a bit different from both of us! :)
One thing that I saw quite a few years back and that initially made me think the code was all commented out, was starting the lines with the ; symbol!
I still find some pascal code now and then that is written like that!
(The reason I thought it was a comment was because at the time I used to code in a multitude of languages. In Assembler, the ; is a common character to start a comment!)
I do like the eyes-friendly formating too and I actively format my sources in an almost identical way... but for 1 point: the "begin..end".
Traditional code formating:
if ThisIsTheTruth then
Begin
DoSomethingSpecial;
For i:=0 to 100 do
Begin
DoSomethingElse;
CheckForNothing;
End;
DoSomethingSpecialAgain;
End;
Suggested code formating:
if ThisIsTheTruth then Begin
DoSomethingSpecial;
For i:=0 to 100 do Begin
DoSomethingElse;
CheckForNothing;
End;
DoSomethingSpecialAgain;
End;
Details:
1) I do write the "begin" on the same line as the starting instruction.
2) Everything else inside the block will be tab-aligned, including the terminal "end"
IMHO, formated that way, the "begin...end" clearly becomes a "child" of the starting instruction (just imagine a treeview).
And I'm able to find/read the instruction following a Begin..end block very easily :)
IOW: It's not only a (my) visual preference, it's also conform to a (my) logical structure.
Note: there's one exception, so far: I do never apply this formating for the method or function declaration itself...
it would waste too much space ;)
Cheers,
L.
GExperts even has a feature (Align Lines) that will align the first example all at once. I use it all the time with long lists of procedures, and constants.
I like your style except for one item: a 'begin' belongs on its own line. There are two reasons for this: it pairs visually with the matching 'end' and it is independent of the controlling line: 'if', 'while', 'with' etc. This allows compounded statements to be copied and moved as 'blocks' of code.
A VERY important issue with coding standards is 'How much format typing is needed if I modify the code in some way'. What if I change a name of a parameter or method name? I would avoid the 'column alignment' like the black plague. JMHO
Michael: thanks for the reminder! I've used GExperts in the past and have been lazy to check whether the new version supported Delphi 2007 (I remember it didn't quite work well when Delphi 2007 got out...)
John: I'd rather have a bit more work when I rename something and go back and re-edit (even without GExperts it's still very easy to make an instant macro to speed up the reformat) rather than loose readability just to save some work. Of course, that's my idea and each developer has their own ideas!
As long as we stick to something that gives us a productivity boost, all is fine, be that either by adapting the code to our way (when we can!) or by adapting our way to the code (when we can't change the formatting or when it would be a far too big a task to be worthwhile).
I learned a long time ago that leaving out Begin/Ends was a forumla for disaster later. Go and add a conditional statement without adding a begin/end later (the brain can hide these little details from us in an attempt to "help") and suddenly code gets flakey indeed.
I always have a begin end.
However, your trailing begins are ALSO a formula for disaster. You can't easily see if they were missed, and frankly they fail my ruler test.
Yup, a RULER test. Print out your code and matching identifiers should be matchable with a straight vertical line with a ruler. If doesn't match End, Begin matches it. (cursoring straight up simulates the ruler test without a printout)
It makes life a lot simpler in the long run - immediate visual recognition of structure prevents you from having to waste hours of your time trying to fix it later when something goes wrong.
Most of my coding style is designed around exactly that - preventing structure problems and making them easier to find if they do happen.
This could seriously turn into a religious debate as to how to align your begin/end pairs. My preference is
if true then
begin
//statements
end;
so that the begin/ends line up, I feel that this makes the code easier to debug. If your from a c/c++ background it is most common to align things
if (true) {
//statements
};
Which most people agree is the easiest to read, but not all - It's worth reading Code Complete 2, Chapter 31 Layout and Style (a truly awesome book - I have a short review on my website). This layout would translate to pascal in the form
if true then begin
//statements
end;
But as always it's a personal taste thing, unless you need to follow corporate code formatting guidelines.
Anonymous: you may have a point with the "ruler" test. However, if you think at two things:
1) I *always* have begin+end;
2) I don't create all that at once, that is, as soon as I type the begin on the end of some line and press enter, Delphi will add the properly indented end and I'll be typing inside.
If you consider those two points, there's no big deal in having the begin on the end. In fact, I find it less distracting, but, as I've said, it's all about what makes *you* more comfortable with the code...
I use coding style
procedure My;
begin
___if ... then begin
______for i := 0 to K do begin
...
______end {for}; //
___end {if};
end {procedure};
1) I always use "begin+end";
2) Every "end" have comment
@John E:
In fact, your remark about "begin belonging on its own line" is not so far off.
Hence, a trade-off would look like:
if ThisIsTheTruth then
|Begin
|DoSomethingSpecial;
|For i:=0 to 100 do Begin
| DoSomethingElse;
| CheckForNothing;
| End;
|DoSomethingSpecialAgain;
|End;
The "|" should demonstrate the alignment i.e. the logical block of code, that could be folded/unfolded...
A supplementary tabbing for the code inside the "begin..end" would be dispensable in that case.
BTW:
When using this formating, you'll hardly need to wrap the "begin..end" block with some comments since the alignment achieves the "visual commitment" ;)
Post a Comment