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