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:
> Hi!
> 
> 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
> 
> Eike

Hey there,

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.

Other points:

* 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.

* ktimer.cpp:

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 mailing list