[NeoStats-Devel] Method in the madness: SeenServ

Just a quick note on the changes I recently made to SeenServ since they may
seem to be change for the sake of change and one of the most important rules
in coding is "if it ain't broke, don't fix it".

1) Introduction of seen_report to replace the multiple checks for message
type:

If channel type
Channel message
Else 
User message

To

Seen_report

This appeared multiple times throughout the code and there was nothing
specifically wrong with this. The seen_report function combines this check
into a single location for the purpose of improving how the code reads and
to ensure the check is done in a single location rather than having the
potential of being missed somewhere. It is thus a simple maintanance change.
If we want to add a new report, we add one line of code instread of four. In
addition, it simplifies potential language support since we reduce the
number of places in the code we need to change by half. 

2) Change of #defines to enum:

The main point of this is that we can perform type checking at compile time.
If you accidentally pass a #define to a function, the compiler cannot tell
and it can make for quite lengthy debugging if something goes wrong. An enum
is as fast as a define at runtime but is type checked by the compiler so you
can be sure your input range is accurate. In addition, by utilising the end
element to establish a count of entries, we can establish dynamic valid
array sizes rather than hardcoded guesses in the code which reduces
maintainance long term.

3) Change of lengthy if else to switch:

Although it may try, a compiler is not very successful at optimising long
and complex if else blocks even when they merely check every case of a given
range. At the machine level they tend compile to multiple branches causing
every else to have an additional branch overhead compared with the first.
E.g.

If I = 1
...code
Elseif I = 2
...code
Elseif I = 3
...code
Endif

Compiles to 

Cmp I,1
Bne else1
...code
Bra end
Else1:
Cmp I,2
Bne else2
...code
Bra end
Else2:
Else If I = 3
...code
:end

I = 1 has almost no cost. All other cases have as many branch checks
necessary to determine their status as true so as I increase, we have an
increasing cost to performance.

However, a switch statement will compile to a jump table meaning all tests
have the same overhead. There is a negligible increase for the first one,
but a noticeable exponential decrease for all others. E.g.

Switch(I)
Case 1:
...code 
break;
Case 2:
...code 
break;
Case 3:
...code 
break;

Compiles to

Jumptable:
Case1
Case2
Case3

Cmp I, numtests
Be end
Bra table[i]
Case1:
...code
Bra end
Case2:
...code
Bra end
Case3:
...code
End:

The initial compare adds to the cost of the first entry in the switch
statement since it is a type check to ensure the range is correct for the
jump table but we had this in the original if anyway. There is a memory
lookup cost to add for the jump table lookup which is where I=1 gets it's
additional overhead. All other tests have the same overhead as this first
test giving us far better performance overall. If testing for every, or most
values from a range, a switch statement will always perform better and is
easier to read than a complex if else if else if system. If you need to test
for multiple conditions, it is not always possible to avoid a lengthy if
else construct, but when testing multiples from a known range of values, the
switch statement will always perform better. 

Compilers constantly improve in their ability to optimise but there is no
harm in helping them do so and given the unknown range of compilers we
support, the traditional hints to a compiler will benefit all users. 

Mark.