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! :)