> (2) Method names in FLTK use some naming conventions that your patches
> should also use. Especially the simple set and get methods do always use
> the same name, with different arguments - so...
>
> void enable_item_shortcuts() and
> void disable_item_shortcuts() should be
> void item_shortcuts(int) where the int argument is 0 or 1 to disable or
> enable item shortcuts, resp..
>
> bool using_item_shortcuts() would then be
> int item_shortcuts() (we're always using int instead of bool for
> historical reasons, however this may change in the future).

I did it that way to show an alternate more descriptive/readable method of 
doing it and a method that doesn't rely on compiler optimizations to prevent 
code bloat.  Figured, easy enough to add existing style of methods if someone 
wanted to since no name collision.

> (4) I'm not a fan of single-character variable names, particularly if the 
> name is 'l' (lower case L) that can easily be confused with 1 (the digit).
> Examples from your patch:
>
> test_shortcut(item_text(l),...
> if ((l=item_next(l))==NULL) { ...
>
>(Please don't say "But they are also elsewhere in the code..."

LOL - but that's the whole reason I used l, to keep it consistant with what was 
already being used.  Outside of loops, I don't use single varaibles myself.


FWIW, I have one folder I do all the changes to, then download a new copy of 
the branch, use winmerge to compare folders, then pull over only the parts I 
want to create a specific patch for posting here.  There's enough changes now, 
that it's getting hard to remember all the little minor changes there may be, 
espically when mixed on top of other patches.
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev

Reply via email to