On Thursday 10 Nov 2005 21:41, Dominik Haumann wrote:
> Hi David,
>
> On Tuesday 08 November 2005 23:00, David Jarvie wrote:
> > Comments please. (Note that I am going away and won't be able to respond
> > for a few days.)
>
> I didn't use the classes, so that's just ideas and questions that arised
> while reading the API.
>
> API inconsistencies:
>  - Sometimes it says "timeZone", sometimes "timezone":
>    KTimezone, setTimeZone, timeZone. Would be good to go only for one of
>    them.

I've commented on this in another posting today.

>  - There is add{Secs, Days, Months, Years} but only secsTo and daysTo.
>    I would actually expect there also is monthsTo and yearsTo.

I have simply copied the QDateTime interface here.

>  - There is dateTime(), but no setDateTime().

The idea is to force the use of operator=(KDateTime) to set the date and time. 
This way, the time zone won't be forgotten about by people who are more used 
to using QDateTime.

> How well does it operate with QDateTime. Does something like
> KDateTime d = QDateTime::currentDateTime() work (defaulting to local time)?

You'd have to use KDateTime d = KDateTime(QDateTime::currentDateTime()) for 
example, for the same reason as above.

> The API documentation looks pretty good. But what I'm missing (through
> whole KDE mostly) are detailed descriptions in the Qt way. Attached is a
> changed class description. I tried to mention all important methods and
> added several sections. It isn't perfect (maybe even wrong in some points)
> as you can guess ;) so you should improve it, but it's a start.

Thanks. That's a useful summary which I'll add in (with any necessary 
modifications/improvements).
>
> Also, cross references (@see) are missing.
> /** [...]
>  *@see daysTo()
> /*
> int secsTo(...) const;
> This could/should be applied to several other methods.

Good point.

-- 
David Jarvie.
KAlarm author and maintainer.
http://www.astrojar.org.uk/linux/kalarm.html