[OpenSER-Devel] SF.net SVN: openser: [3442] trunk

[Henning Westerholt]

On Wednesday 19 December 2007, Dan Pascu wrote:
> What does this help? A constant like MAX_FD should be a define in a global
> header file, something like config.h which can be included by any source
> code that needs it. When it needs to change, one need only edit 1 single
> line in a single file.

Well,

this #define was only used in one place. So i see no benefit to have this 
defined global.

> As it was I could still grep by MAX_FD and change the 3 occurrences it
> would find. But as it is now, what should I grep for? 32? It gives 588
> hits!
>
> Also what is the point of using a local variable constrained to a constant
> in place of a #defined constant?

Decreasing the variable scope. A #define is visible global. This define was 
only used in one function, so i do not see any benefit to export this for the 
whole server. One long term goal of mine is to document global variables for 
the server, if i could eliminate this, then i do not need to write 
documentation for this. Defines are also not typesafe at all.

> IMO, #defined constants at the top of a source file, are a simple way for
> someone who is not familiar with the code to modify the outcome of the
> code by altering simple constants that are found at the top of the file
> without dwelling into the source code. As you modified, one has to dig
> into the source code just to find where something as simple as a constant
> needs to be changed.

If the define is not documented, then one could not simply change them. One 
must review the code that uses this too. I've also added a comment that shows 
the purpose of the constant. The timeout define was also not placed on top of 
the file at all.

Otherwise i agree with you. And i don't plan to replace all defines with 
variables, or remove them all.

If you are really not satisfied with my cleanup work, then i will stop this. 
Otherwise please lets not fight about this ~10 lines change. I also need to 
get some work done for today, so please lets stop the discussion here.

Thank you,

Henning