Re: [gnome-db] New patch

[Rodrigo Moya <rodrigo gnome-db org>]

On Thu, 2002-01-10 at 04:53, Gonzalo Paniagua Javier wrote:
> 	In the .tgz attached there are 4 files with patches for
> 	testing/, libgda/ idl/ and providers/postgres/.
> 
> 	They include the date, time and timestamp stuff, the
> 	support for them under postgres, changes to gda-row.[ch] and
> 	gda-value.[ch] about 'const'ness, removed lots of warnings...
> 
some things are wrong. You've added const to the GdaField parameter in
the get_ functions. There is no need for doing so. What you should do is
to add const for the return values where appropriate, but not to the
GdaField paramerter. What is also const are the second arguments to the
set_ functions, but only for pointers. There's no need for doing 'const
gint' or something similar.

Also:

> -GDate *
> -gda_field_get_date_value (GdaField *field)
> +const GdaDate *
> +gda_field_get_date_value (const GdaField *field)
>  {
>         g_return_val_if_fail (field != NULL, NULL);
> -       g_return_val_if_fail (
> -               field->attributes.gdaType == GDA_TYPE_DATE,
> -               NULL);
> 
>         return gda_value_get_date (&field->value);
>
why are you removing that check?

> -GTime
> -gda_field_get_time_value (GdaField *field)
> +const GdaTime *
> +gda_field_get_time_value (const GdaField *field)
> {
> -       g_return_val_if_fail (field != NULL, -1);
> -       g_return_val_if_fail (
> -               field->attributes.gdaType == GDA_TYPE_TIME,
> -               -1);
> +       g_return_val_if_fail (field != NULL, NULL);
>
also, don't remove that check.

> -time_t
> -gda_field_get_timestamp_value (GdaField *field)
> +const GdaTimestamp *
> +gda_field_get_timestamp_value (const GdaField *field)
>  {
> -       g_return_val_if_fail (field != NULL, -1);
> -       g_return_val_if_fail (
> -               field->attributes.gdaType == GDA_TYPE_TIMESTAMP,
> -               -1);
> +       g_return_val_if_fail (field != NULL, NULL);
>
the same, don't remove the checks for the type

> 	gda-value.c still has a few warnings related to ORBit functions
> 	(ORBit_RootObject_duplicate() and CORBA_Object_release()).
> 	Somebody, please, take a look at them.
>
this part looks good, except for the extra const you've added in some
places.
I've only applied the idl part, so please resend the libgda part. The
postgres and test parts look good, but haven't applied them as they need
the changes in libgda. So please resend that part (along with the
postgres and testing patches), and if it is ok, you can apply yourself
with your new CVS account :-)
 
> 	My TODO list:
> 
> 		-gda-test: display the GdaRecordset contents (don't know
> 		how!).
> 
in testing/model.c there's a function (display_data_model or something
like that which will do it for you). Also, the schemas function in
client.c contain some code you can use as example.
BTW, could you also move the postgres-specific parts in client.c to a
separate file (postgres.c)? This will make the code cleaner, and then we
can add new files for provider specific tests.

cheers
-- 
Rodrigo Moya <rodrigo gnome-db org> - <rodrigo ximian com>
http://www.gnome-db.org/ - http://www.ximian.com/