|
|
| Line 28: |
Line 28: |
| | | | |
| | See also [[ReactOS Remote Debugger]] --[[User:Lone Rifle|Lone Rifle]] 09:19, 27 August 2009 (UTC) | | See also [[ReactOS Remote Debugger]] --[[User:Lone Rifle|Lone Rifle]] 09:19, 27 August 2009 (UTC) |
| − |
| |
| − | == Coding style - What's good or bad ? ==
| |
| − |
| |
| − | Real life sure takes it's toll, but here I am again.
| |
| − | During the past months I've read code included in this project when I had some time,
| |
| − | and found a lot of, in my opion, bad code style.<br>
| |
| − | This is a subject that has been covered over and over again, on most available forums of discussion,
| |
| − | and yet it seems that it still could do with more consideration.<br>
| |
| − | I will therefore give two examples of bad style, and motivate why I think it's bad.<br>
| |
| − | Then I will suggest improvements that increase readability and clarity.<br>
| |
| − | <br>
| |
| − | <b>Example 1:</b> (Common style in f.ex WINNT.H)
| |
| − | <br>
| |
| − | NTSYSAPI
| |
| − | VOID
| |
| − | NTAPI
| |
| − | RtlZeroMemory (
| |
| − | VOID UNALIGNED *Destination,
| |
| − | SIZE_T Length
| |
| − | );
| |
| − | <b><i>Why is it bad?</i></b><br>
| |
| − | <br>
| |
| − | a) It separates the function prefixes to separate lines,
| |
| − | which makes reading it very unnatural.<br>
| |
| − | b) It separates the arguments to separate lines, even though
| |
| − | they aren't followed by line comments clarifying the aguments,
| |
| − | and they don't need line breaks to decrease line length.<br>
| |
| − | c) It doesn't include argument direction hints.<br>
| |
| − | <br>
| |
| − | <b><i>What is better?</i></b><br>
| |
| − | <br>
| |
| − | NTSYSAPI VOID NTAPI
| |
| − | RtlZeroMemory (
| |
| − | IN VOID UNALIGNED *Destination, // Destination address to fill
| |
| − | IN SIZE_T Length // Size, in bytes, of memory block
| |
| − | );
| |
| − | or:<br>
| |
| − | <br>
| |
| − | NTSYSAPI VOID NTAPI
| |
| − | RtlZeroMemory (
| |
| − | IN VOID UNALIGNED *Destination,
| |
| − | IN SIZE_T Length
| |
| − | );
| |
| − | or simply:<br>
| |
| − | <br>
| |
| − | NTSYSAPI VOID NTAPI
| |
| − | RtlZeroMemory ( IN VOID UNALIGNED *Destination, IN SIZE_T Length );
| |
| − | Note that in no case does the line length exceed a nominal 80 chars.<br>
| |
| − | <br>
| |
| − |
| |
| − | <b>Example 2:</b><br>
| |
| − |
| |
| − | case IDM_EDITPASTE:
| |
| − | OpenClipboard(hMainWnd);
| |
| − | if (GetClipboardData(CF_BITMAP) != NULL)
| |
| − | {
| |
| − | DeleteObject(SelectObject(hSelDC, hSelBm = CopyImage(GetClipboardData(CF_BITMAP),
| |
| − | IMAGE_BITMAP, 0, 0,
| |
| − | LR_COPYRETURNORG)));
| |
| − | newReversible();
| |
| − |
| |
| − | <b><i>Why is it bad?</i></b>
| |
| − |
| |
| − | a) Omitting space between the argument list parentheses and the arguments makes it very difficult to read the code.<br>
| |
| − | b) Aligning arguments with the opening parenthesis of the argument list makes the lines ridiculusly long,
| |
| − | which puts the text well beyond the right margin of the editor, makes it very difficult to read,
| |
| − | causes excessive eye movement and excessive horizontal scrolling, and doesn't leave any room for line comments.<br>
| |
| − | c) Embeds an assignment operator in the argument list even though it improves neither efficiency nor readability.<br>
| |
| − |
| |
| − | <b><i>What is better?</i></b>
| |
| − |
| |
| − | case IDM_EDITPASTE:
| |
| − | OpenClipboard( hMainWnd );
| |
| − | if (GetClipboardData( CF_BITMAP ) != NULL )
| |
| − | {
| |
| − | hSelBm = CopyImage(
| |
| − | GetClipboardData( CF_BITMAP ),
| |
| − | IMAGE_BITMAP, 0, 0, LR_COPYRETURNORG
| |
| − | );
| |
| − | DeleteObject( SelectObject( hSelDC, hSelBm ));
| |
| − | newReversible();
| |
| − |
| |
| − | <b><i>Why is it good?</i></b><br>
| |
| − |
| |
| − | a) It inserts a space between the argument list parentheses and the arguments.<br>
| |
| − | b) It indents the arguments a single step, instead of aligning them with the argument list opening parenthesis,
| |
| − | which makes the text easier to read, and leaves room for line comments.<br>
| |
| − | c) It puts the assignment operator outside the argument list, which greatly improves both readability and clarity.<br>
| |
| − | <br>
| |
| − | This is not intended to be dogmatic absolutes, but I hope it gives You something to think about,
| |
| − | and hopefully induce You to write clearer code.<br>
| |
| − | Rock on <br>
| |
Welcome!, your skills could be of help here, please visit #reactos to get in touch with the team and have a chat about how to get started, in case you haven't already done so... :) -- gabriel_it
Of course, he meant #reactos on Freenode IRC =) . There's also the ros-dev mailgroup which can be subscribed from here. -- Lone Rifle 09:26, 23 July 2009 (UTC)
Yeah, that's what I meant ;). -- gabriel_it
Thank You. Of course, amigos.. I will show my prompt on #reactos from time to time :)
27 Aug 2009: Last few weeks I was busy building and rebuilding machines for my local net.
Finally, I got me a box fast enough to run VMware without wearing out my razor blades,
if you know what I mean ;^)
Now I got to figure out how to set up VMware for kernel debugging of ReactOS.
I should be possible to use a named pipe redirection of a COM port in the VM,
and a corresponding virtual COM port on the host's end of the pipe.
I'll just have to find that last bit. I don't wanna blow a whole lot of effort
writing and debugging that part if there already is one.
If You have experience of this issue, don't hesitate to post me a hint, amigo..
Hi, I know what you mean ;) welcome back!
These will probably be of use:
[[1]]
[[2]]
[[3]]
[[4]]
Good luck! -- gabriel_it 08:52, 27 August
See also ReactOS Remote Debugger --Lone Rifle 09:19, 27 August 2009 (UTC)
