[OpenSER-Devel] database module issues (rant)

[Daniel-Constantin Mierla]

Hello Henning,

good points. Would you please add the notes to dokuwiki, in development 
section, maybe under something like "Developing a new DB module" -- 
would be easy to refer to it and add improvements to the rules in the 
future.

db_text needs a deep review for quite some time now, but as I am not 
using it that often, got all the time very low priority in my list and 
nobody else wanted to jump in :) . I will try to find some time though.

Thanks for DB rework and thorough analysis, the layer got lot of more 
consistency in structure and naming format.

Cheers,
Daniel


On 02/07/08 13:49, Henning Westerholt wrote:
> Hi all,
>
> some thoughts about issues that i've encountered during my journey through the 
> database modules. Sorry about this rant, but it's a little bit tiring to 
> going through this issues again and again. So if you think about writing a 
> new database module, here is a list with some points to check against.. ;-)
>
>
> 1. Error checking
>
> I think its absolut necessary to check for the validity of your input 
> parameter. Otherwise you'll crash on the first null pointer that you get. 
> The job of the database modules is to provide a API for other modules, thus 
> offer a service. The client code will be invetable try to do stupid things 
> because we're not living in a perfect world.
>
> So please don't hide your input checks behind some #ifdefs 
> like "XXX_EXTRA_DEBUG" or something similar. In general there should be no 
> new #ifdefs introduced for (in my opinion stupid) things like this. Nobody 
> will configure them, instead the default will be used. Every #ifdef increase 
> the complexity, the number of code paths that needs to be tested, in the end 
> the number of bugs. So again, instead of shifting the responsibility to the 
> user or packager, try to find a solution that works generally.
>
>
> 2. Error logging
>
> Its really essential to output meaningful error messages with the correct log 
> level for problems that causes operations to fail. Its not feasible to run a 
> production server with activated debug only to get information about the 
> reason why this certain query failed. Error logs provides important 
> information that is needed from API users, so please use LM_ERR and don't 
> hide them under thousands of other meaningless debug messages. So again, if 
> you return a failure to the client of the API, log also an error with a 
> meaningful message that is understandable by the admin or user that don't 
> wade through the code regularly. 
>
>
> 3. Code structure
>
> Maintaining all this bunch of modules is much more easier if everybody tries 
> to conform to a common coding style and structure. So please use the same 
> conventions in your code that are used in other database modules, even if you 
> don't like them. This applies to function definitions, parameter names, error 
> logging, debug messages, even to the practise of memory management.
>
> To be fair, this is only possible to a certain degree, but e.g. all SQL based 
> driver library interfaces are somewhat similar, there was no reason in the 
> first place why the SQL modules should differ that much. An additional 
> benefit in having the same error and debug messages in different modules is 
> that this really makes debugging and the bug fixing process easier. I know 
> that you only care about "your" driver during development, but you should 
> also care about the whole project. 
>
>
> 4. Documentation
>
> Yes, nobody likes to write documentation. But the absolute minimum to 
> understand any non-trivial foreign code is to have a comment block at the 
> function definition describing in a few sentence the purpose of the code, the 
> input parameter and return value. Its also important to describe why you do 
> certain things in your code, e.g. because of some driver requirements, some 
> bugs etc.. No comments in the code slows down the maintainance, more bugs 
> will be introduced on changes and so on.
>
>
> If we (as project) want to grow further in the future, we should seriously 
> think about the conception of some coding standards and best practises in 
> development, as most of this points also applies to other modules. Otherwise 
> the code base will at some point in the future collapse under their own 
> wight.
>
>
> Best regards,
>
> Henning
>
>
> P.S: Code examples for the respective issues can be found at:
>
> 1. db_text and db_berkeley
> 2. db_text: dbt_base.c
> 3. db_mysql, db_unixodbc vs. db_postgres, now fixed
> 4. db_text and db_berkeley
>
> _______________________________________________
> Devel mailing list
> Devel at lists.openser.org
> http://lists.openser.org/cgi-bin/mailman/listinfo/devel
>
>