KTimer, Patch for wish 195647

Raphael Kubo da Costa kubito at gmail.com
Sat Apr 17 01:27:06 CEST 2010


On Friday 16 April 2010 08:08:30 Eike Krumbacher wrote:
> Hi Again!
> OK, removed the Tab/Saces thing as good as I can.
There's still

-		m_commandLine->disconnect();
-		m_commandLine->lineEdit()->disconnect();
+        m_commandLine->disconnect();
+        m_commandLine->lineEdit()->disconnect();

> I haven't found a QSpinBox::valueChanged() Signal without parameter at
> http://doc.trolltech.com/4.6/qspinbox.html
You need not match all the parameters from a signal. The following is also 
correct:

connect(foo, SIGNAL(frob(int)), bar, SLOT(frobbed()));

Some additional comments:

+    return QString( "%1:%2:%3" ).arg( h ).arg( m,2, 10, QChar( '0' ) ).arg( 
s,2, 10, QChar( '0' ) );

Coding style-wise, I'd rather have "m, 2" and the like instead of "m,2" here.

+int KTimerJob::timeToSeconds( int hours, int minutes, int seconds ) const
+{
+    int s = hours * 3600 + minutes * 60 + seconds;
+    return s;
+}

Why not simply "return hours * 3600 + minutes * 60 + seconds"?

+void KTimerJob::secondsToHMS( int secs, int& hours, int& minutes, int& 
seconds ) const

In general, when following Qt's and KDE's coding style, we don't take 
parameters by reference and change them.

If hours, minutes and seconds are going to be changed, it's better to declare 
them as pointers, so that the caller needs to pass their addresses and is 
aware that they are going to be changed.

Cheers,
Raphael


More information about the Kde-utils-devel mailing list