Re: [pdf-devel] Changes in the Time module API

[Aleksander Morgado]

>    I revised the Time module API and I propose the changes attached to the
>    email. Basically:
>
>    - Set as `const' those functions parameters that won't be modified in 
>    the function call, to easily detect which are input parameters and which 
>    are output parameters.
>
> It is a good practice. I think that would be good to apply it in the
> existing modules too. I will open a low-priority task for this.

Perfect!

>    - Calendar type stores month/day... in 32bit variables. Probably 8 bits
>    are enough for these fields (not for `year', which remains in
>    32bit).
>
> To use 32bit words to store these values would improve the efficiency
> of the library, if running in usual 32bit machines. My feeling is that
> we should use 32bit integers even if we are wasting space. It is not a
> strong feeling anyway.

Ok, not a big issue anyway.

>    - Changed `pdf_i32_t' name to `pdf_32_t', as defined in pdf_types.h.
>    BTW, which one is better? I prefer `pdf_32_t', but I could change it.
>    `pdf_u32_t' remains as is.
>
> I much prefer the `pdf_i32_t' name, since it makes explicit that the
> stored value is a (signed) integer.

I bet yesterday with Daniel that you would say that :-). Ok, I will 
change the pdf_32_t with pdf_i32_t in the whole library (text module 
basically).

>    - Those functions filling in output `struct pdf_time_cal_span_s' and
>    `struct pdf_time_cal_s' variables should manage pointers to the
>    structures (passing by reference the structure in order to be able to
>    modify its contents).
>
> Yes. It is a bug in the API.

Ok.

>    - Changed the name of the pdf_time_t variable passed to the functions
>    from `time' to `object', to avoid using the same name as the `time()'
>    function.
>
> Hm? `time' is more explicit. Being an actual parameter, where is the
> confusion with time()?

Just didn't want to make the compiler unhappy if `time()' is actually 
used in a function receiving a variable named `time', which could really 
happen in this module.

>    - The function `pdf_time_span_to_secs' was returning a 32bit value to 
>    represent the time span in seconds. As the time span is stored as a 
>    64bit value internally, it should return a pdf_64_t.
>
> Another bug in the API :)

Ok.


-Aleksander