Re: Fw: [hatari-devel] Hatari and keymap

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On 4/28/20 8:45 AM, Thomas Huth wrote:
> I just noticed that you've sent this mail only to me ... Please
> always send patches to the list, and not individual developers. I'm
> forwarding it to hatari-devel in case Nicolas or Eero or somebody
> else want to have a look at the patch...
>
> Some things I noticed after having a quick look:
>
> +# Syntaxe : <SDL Key>[-<SDL Key Mod>],<ST Scancode>[-<ST Scancode>[...]]
>
> "Syntax" is written with "e" in english ... and what about that "@"
> character that you use in your examples? You don't mention it in the
> description there?
>
> +#define MAX_KEYSYM_VALUE 255
>
> The keysyms are definitily not limited to 255 in SDL2, so that sounds
> wrong.

Also, for sizeof, we use sizeof(), not defines:
------------
+#define MAX_KEYSYM_VALUE 255
+#define SIZEOF_KEYSYM_ARRAY (MAX_KEYSYM_VALUE + 1)
+#define SIZEOF_KEYPOS_ARRAY 256
------------

Hatari code typically works like this:
------------
#define KEYSYM_ITEMS 256
int keysyms[KEYSYM_ITEMS];
for (i = 0; i < ARRAY_SIZE(keysyms); i++)
{
	keysym[i] = ...
------------

(ARRAY_SIZE is defined in main.h)


> All in all, I'm also still having a hard time to read your new code.
> Maybe you could split it up logically in multiple patches? Or at least
> refactor the parts with a lot of indentation into new functions?

I think both are needed, couple of separate
patches, and splitting the huge function(s).


Here's what e.g. kernel coding style says of functions:
-------------------------------------------------
Functions should be short and sweet, and do just one thing. They should 
fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, 
as we all know), and do one thing and do that well.

The maximum length of a function is inversely proportional to the 
complexity and indentation level of that function. So, if you have a 
conceptually simple function that is just one long (but simple) 
case-statement, where you have to do lots of small things for a lot of 
different cases, it’s OK to have a longer function.

However, if you have a complex function, and you suspect that a 
less-than-gifted first-year high-school student might not even 
understand what the function is all about, you should adhere to the 
maximum limits all the more closely. Use helper functions with 
descriptive names.

Another measure of the function is the number of local variables. They 
shouldn’t exceed 5-10, or you’re doing something wrong. Re-think the 
function, and split it into smaller pieces. A human brain can generally 
easily keep track of about 7 different things, anything more and it gets 
confused. You know you’re brilliant, but maybe you’d like to understand 
what you did 2 weeks from now.
-------------------------------------------------


	- Eero