KTimer, Patch for wish 195647
Raphael Kubo da Costa
kubito at gmail.com
Fri Apr 16 04:15:55 CEST 2010
On Thursday 15 April 2010 12:59:30 Eike Krumbacher wrote:
> In bug report 195647, the reporter wishes to enter the time in day,
> hour, minutes and seconds, just to not calculate the time in seconds
> using a calculator.
> The attached patch follows this idea. There were two QSpinButtons added,
> one for hour and one for minutes. I think, that "days" is much too far
> in the future, but if you like, I would add this spin box too.
> The signals are new connected, the time is displayed in
> hour:minutes:seconds-format in the table. Furthermore, there is some
> code cleanup, as I removed tabs and replaced them by spaces.
> Please note, that I did not touch the LCD-Widget. This displays the
> seconds until job-start.
> OK, thats it
The patch looks quite big. I'd rather have at least two separate ones -- one
for the tabs/spaces stuff, and another one implementing the new functionality.
* ktimer.h: QString formatTime(const int seconds) const;
The parameter does not need to be const, as it is a primitive type.
Furthermore, it should have spaces between the brackets and the parameters
according to the rest of the file.
You seem to do the same arithmetic for hours, minutes and seconds in at least
three different places in the file. Isn't it better to have a QTime object in
the class for that?
You don't use the int parameter in changeDelay -- you can simply remove it and
connect changeDelay() to a SIGNAL(valueChanged()). And since it is a slot,
it'd be good to follow the naming convention and call it delayChanged() or
something like that.
More information about the Kde-utils-devel