[vlc-devel] [PATCH] Sqlite Module for VLC, GSoC 09

[Laurent Aimar]

Hi,

On Sat, Aug 29, 2009, Srikanth Raju wrote:
> +struct sql_t
> +{
> +    VLC_COMMON_MEMBERS
> +
> +    /** Module properties */
> +    module_t  *p_module;
> +
> +    /** Internal data */
> +    sql_sys_t *p_sys;
> +
> +    /** Set parameters for the database connection */
> +    void (*pf_set_database) ( sql_t *, const char *,
> +                              int , const char *, const char * );
> +
> +    /** Connect to the database previously configured */
> +    int (*pf_open_database) ( sql_t * );
> +
> +    /** Close the connection */
> +    int (*pf_close_database) ( sql_t * );

 Is so many functions really needed ?

 IMHO, merging pf_set_database and pf_open_database in the module Open
and pf_close_database in the module Close function would simplify it
without any loss. It would also prevent stupid usage mistakes.

> +
> +    /** Perform a query with a row-by-row callback function */
> +    int (*pf_query_callback) ( sql_t *, const char *,
> +            int (*callback) ( void*, int, char**, char** ), void * );
> +
> +    /** Perform a query and return result directly */
> +    int (*pf_query) ( sql_t *, const char *, char ***, int *, int * );
> +
> +    /** Get database tables */
> +    int (*pf_get_tables) ( sql_t *, char *** );
> +

> +    /** Free result of a call to sql_Query or sql_GetTables */
> +    void (*pf_free) ( sql_t *, char ** );
 I am not sure this one is really needed. Couldn't it be made independant
of the module implementation ?

> +
> +    /** vmprintf replacement for SQL */
> +    char* (*pf_vmprintf) ( const char*, va_list args );

> +    /** Begin transaction */
> +    int (*pf_begintransaction) ( sql_t* );
> +
> +    /** Commit transaction */
> +    void (*pf_committransaction) ( sql_t* );
> +
> +    /** Rollback transaction */
> +    void (*pf_rollbacktransaction) ( sql_t* );
 Is the separation into 4 functions really needed ? And if so why ?

Regards,

-- 
fenrir