Win32Lib

new topic     » goto parent     » topic index » view thread      » older message » newer message

Just a few random notes on Win32Lib. Jiri, you can ignore everything until
the last section, where I start complaining about how code should be
indented. At that point, feel free to mock me. blink

1. I got a note from someone who had trouble with the initCommonControls
routine. I don't have documentation on the routine, but I'm guessing that
it's failing because he doesn't have things like the coolbar and such
installed on his machine. These are typically installed when a person
installs IE (one of Microsoft's many abhorant practices).

I'm a bit leery about requiring things like this to be on a machine, even if
they are fairly standard. Should Win32Lib really fail if a non-baseline
libraries are not included? Much of the world (myself included) continues to
use Win95.

Perhaps it would be better if, if the routine failed, it set a flag so that
dependant classes would issue a warning. That way, you could run Win32Lib
applications, even if you didn't have the complete set of Windows bells and
whistles on your machine.

2. Passing the font color to a RichText control by using { color, FaceName }
in the setFont routine seems to me a Bad Idea. It really should use the
color set in setTextColor.

3. The LimitText routine should probably be called setTextLimit, to make it
more consistant with the other routine names.

4. If you really want tab_direction is to be made global (do you?), it
should probably be with the global wrappers setTabForward() and
setTabBack().

5. I'm too lazy to test this - does setHintFont still work? I _suspect_ that
it's now broken, and can be removed, along with associated variables that
are still scattered throughout the code.

6. What's the purpose of hitTestTV? I mean, for the general user?


Now the issue of coding style. I didn't intend to get onto this subject, but
I ran into a block of code that looks very, very different from the code
around it. This isn't to say that the code doesn't work, or the coding style
is inherently wrong. But it's distractingly different, and I think that it
would be a Good Thing if the library code adhered to a common coding style.

Obviously, I'm biased toward my own coding style, and I think that it's the
One True Way. And I'd far and away prefer code that doesn't follow the rules
to people deciding not to add to the project because they don't like my
coding style.

Finally, I'm no longer in charge of the project, so these are just requests,
not requirements!

With those caveat in mind, here are some examples where other coders differ
from me:

   sequence vLMB_id
   vLMB_id = {-1, -1, -1, -1}

   atom
         vMaxClickTime
   integer
         vMaxClickXDelta
       , vMaxClickYDelta

   vMaxClickTime   = 0.5
   vMaxClickXDelta = 3
   vMaxClickYDelta = 3

It's too bad Euphoria doesn't have the concept of persistant variables in
routines, since it leads to code like this (and I'm just as bad as anyone
else). These variables should be placed *immediately* before the routine
that they are used in. In the above example, they precede wmScroll, but are
actually used in setMouseClick.

*Always* add comments explaining what groups of variables are, no matter how
obvious they are to you, along with your name, and the version of Win32Lib
that you are altering. That way, if people report that code added after some
particular release breaks things, it becomes relatively easy to figure out
what code is suspect.

And please, write put the commas in constant declarations *immediately*
after the expression, *not* on the next line:

   integer
         vMaxClickXDelta,
         vMaxClickYDelta

Yes, I know that it makes it more difficult to comment out, but it makes the
code *much* more readable.

More code snippets. Again, the coding isn't intrinsically bad, it just
doesn't match my own style (which is 60% of the library):

   ----------------------------------------------------
   function fDoMouse(integer id, atom hWnd, atom iMsg, atom wParam, ...
   ----------------------------------------------------
   integer
         mouseX
       , mouseY
       , action
   sequence
        lRC

      lRC = {pReturn}

Here's my rewrite.

   ---( 80 chars wide )---------------------------------------------
   function fDoMouse(integer id, atom hWnd, atom iMsg,
                      atom wParam,  ... )

      -- Add comments explaining what the function does here!

      integer
         mouseX,   -- mouse x position
         mouseY,   -- mouse y position
         action       -- mouse action

If the code doesn't fit on 80 lines, break it (the API linking routines are
the exception).  This makes it easy to scan the code, and find the start of
each routine. If the routine is global, the explanation of the routine comes
immediately after.

I should explain here a bit: I initially placed documentation comments
*inside* the code that was being documented, but it wasn't a good match, and
made it difficult to see where the routine actually began. So I decided that
the documentation comments should immediately preceed the routine they
document, and that seems to work fairly well.

If the routine is not global, a short explanation of the routine is placed
after the routine declaration, and before the variable declaration.
Everything in the routine - comments, variable declarations, etc. is
indented, so you can clearly see they are part of the routine.Comments
following declared variables, no matter how banal, are appreciated.

In a compound functions, like this:

   if iMsg = WM_LBUTTONUP
   then -- test for a "click" gesture.
     if vLMB_id[1] = id
           and
         time()-vLMB_id[4] < vMaxClickTime
           and
         abs(mouseX - vLMB_id[2]) < vMaxClickXDelta
           and
         abs(mouseY - vLMB_id[3]) < vMaxClickYDelta
     then -- Clicking! Any handler for this?

the 'and' and 'or' clauses are aligned with the prior 'if':

   if iMsg = WM_LBUTTONUP then

     -- test for a "click" gesture.
     if vLMB_id[1] = id
     and time()-vLMB_id[4] < vMaxClickTime
     and abs(mouseX - vLMB_id[2]) < vMaxClickXDelta
     and  abs(mouseY - vLMB_id[3]) < vMaxClickYDelta then

         -- Clicking! Any handler for this?

Comments are placed just above the code they refer to, not after them.
Otherwise, it's too tempting to make the comments even shorter and more
cryptic than they already are.

If three lines of code go by without a comment, you're not commenting
enough! Remember, the more comments you add, the more likely that someone
else will be able to find and fix a bug for you!

Finally, a note about changes. Whenever I made changes to the file, I'd add
a comment (flush with the left margin, so it was easy to see) like this:

   -- NEW! v0.51

I used NEW! because that gave me a keyword to search the file when I wanted
to remove all comments prior to some given release. The version number (as I
mentioned before) is extremely helpful in figuring out if the change might
be related to some newly reported bug.

OK, enough whining about style. If someone wants to explain why I'm wrong,
I'll be glad to hear it.

Thanks again to everyone who's participating in this project!

-- David Cuny

new topic     » goto parent     » topic index » view thread      » older message » newer message

Search



Quick Links

User menu

Not signed in.

Misc Menu