Merge lp:~nik90/ubuntu-clock-app/migrate-stopwatch-utils-c++ into lp:ubuntu-clock-app

[Nekhelesh Ramananthan]

Converted StopwatchUtils.qml functions to C++ functions to shave loading times and improve performance by a tiny bit.

This MP converts the StopwatchUtils.qml functions to C++ functions for the following reasons,

When I ran stopwatch using Qt Profiler, I noticed that StopwatchUtils{} takes around 60 ms to load, and each function call within takes an average of 200 microsecond. On converting this to c++, the load time reduces to 500 microseconds, so from 60ms->500microsecond! And each function call takes half the time (100 microsecond)

While the performance shaving are quite small, it raises the question as to why not do this change? I see no disadvantages. Nothing in the qml side needs to be changed, unit tests pass. Regression free!

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:363
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/811/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/156

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/811/rebuild

review: Approve (continuous-integration)

[lp:~nik90/ubuntu-clock-app/migrate-stopwatch-utils-c++]

364. By Nekhelesh Ramananthan on 2015-08-25

Changed StopwatchUtils to just Utils

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:364
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/812/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/157

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/812/rebuild

review: Approve (continuous-integration)

[lp:~nik90/ubuntu-clock-app/migrate-stopwatch-utils-c++]

365. By Nekhelesh Ramananthan on 2015-08-26

Renamed Utils to FormatTime

366. By Nekhelesh Ramananthan on 2015-08-26

Fixed hours being truncated when they are greater than 99

367. By Nekhelesh Ramananthan on 2015-08-26

Fixed unit tests

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:366
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/814/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/159/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/814/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:367
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/815/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-clock-app-vivid-amd64-ci/160

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/815/rebuild

review: Approve (continuous-integration)

[Bartosz Kosiorek (gang65)]

works perfectly for me, and everrything is fine according to checklist

review: Approve

=== modified file 'app/stopwatch/CMakeLists.txt'
--- app/stopwatch/CMakeLists.txt	2015-08-09 20:59:10 +0000
+++ app/stopwatch/CMakeLists.txt	2015-08-26 22:51:08 +0000
@@ -2,7 +2,6 @@
     LapListView.qml
     StopwatchFace.qml
     StopwatchPage.qml
-    StopwatchUtils.qml
 )
 
 # make the files visible in the qtcreator tree

=== modified file 'app/stopwatch/LapListView.qml'
--- app/stopwatch/LapListView.qml	2015-08-20 19:59:21 +0000
+++ app/stopwatch/LapListView.qml	2015-08-26 22:51:08 +0000
@@ -19,14 +19,15 @@
 import QtQuick 2.4
 import QtQuick.Layouts 1.1
 import Ubuntu.Components 1.2
+import Stopwatch 1.0
 
 ListView {
     id: lapListView
 
     clip: true
 
-    StopwatchUtils {
-        id: stopwatchUtils
+    StopwatchFormatTime {
+        id: stopwatchFormatTime
     }
 
     header: ListItem {
@@ -108,11 +109,11 @@
                 Row {
                     anchors.horizontalCenter: parent.horizontalCenter
                     Label {
-                        text: stopwatchUtils.lapTimeToString(model.laptime) + "."
+                        text: stopwatchFormatTime.lapTimeToString(model.laptime) + "."
                     }
                     Label {
                         fontSize: "x-small"
-                        text: stopwatchUtils.millisToString(model.laptime)
+                        text: stopwatchFormatTime.millisToString(model.laptime)
                         anchors.bottom: parent.bottom
                         anchors.bottomMargin: units.dp(1)
                     }
@@ -125,11 +126,11 @@
                 Row {
                     anchors.horizontalCenter: parent.horizontalCenter
                     Label {
-                        text: stopwatchUtils.lapTimeToString(model.totaltime) + "."
+                        text: stopwatchFormatTime.lapTimeToString(model.totaltime) + "."
                     }
                     Label {
                         fontSize: "x-small"
-                        text: stopwatchUtils.millisToString(model.totaltime)
+                        text: stopwatchFormatTime.millisToString(model.totaltime)
                         anchors.bottom: parent.bottom
                         anchors.bottomMargin: units.dp(1)
                     }

=== modified file 'app/stopwatch/StopwatchFace.qml'
--- app/stopwatch/StopwatchFace.qml	2015-08-17 20:27:12 +0000
+++ app/stopwatch/StopwatchFace.qml	2015-08-26 22:51:08 +0000
@@ -17,6 +17,7 @@
  */
 
 import QtQuick 2.4
+import Stopwatch 1.0
 import Ubuntu.Components 1.2
 import "../components"
 
@@ -29,8 +30,8 @@
     isOuter: true
     width: units.gu(32)
 
-    StopwatchUtils {
-        id: stopwatchUtils
+    StopwatchFormatTime {
+        id: stopwatchFormatTime
     }
 
     ClockCircle {
@@ -46,13 +47,13 @@
         anchors.centerIn: parent
 
         Label {
-            text: stopwatchUtils.millisToTimeString(milliseconds, false, true)
+            text: stopwatchFormatTime.millisToTimeString(milliseconds, true)
             font.pixelSize: units.dp(34)
             color: UbuntuColors.midAubergine
         }
 
         Label {
-            text: stopwatchUtils.millisToString(milliseconds)
+            text: stopwatchFormatTime.millisToString(milliseconds)
             font.pixelSize: units.dp(18)
             color: UbuntuColors.midAubergine
             anchors.horizontalCenter: parent.horizontalCenter

=== modified file 'app/stopwatch/StopwatchPage.qml'
--- app/stopwatch/StopwatchPage.qml	2015-08-18 13:13:32 +0000
+++ app/stopwatch/StopwatchPage.qml	2015-08-26 22:51:08 +0000
@@ -17,8 +17,8 @@
  */
 
 import QtQuick 2.4
+import Stopwatch 1.0
 import Ubuntu.Components 1.2
-import Stopwatch.LapHistory 1.0
 
 Item {
     id: _stopwatchPage

=== removed file 'app/stopwatch/StopwatchUtils.qml'
--- app/stopwatch/StopwatchUtils.qml	2015-08-17 20:27:12 +0000
+++ app/stopwatch/StopwatchUtils.qml	1970-01-01 00:00:00 +0000
@@ -1,73 +0,0 @@
-/*
- * Copyright (C) 2015 Canonical Ltd
- *
- * This file is part of Ubuntu Clock App
- *
- * Ubuntu Clock App is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 3 as
- * published by the Free Software Foundation.
- *
- * Ubuntu Clock App is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-import QtQuick 2.4
-import Ubuntu.Components 1.2
-
-/*
-  Qt Object containing a collection of useful stopwatch functions
-*/
-QtObject {
-    id: stopwatchUtils
-
-    // Function to return only the milliseconds
-    function millisToString(millis) {
-        return addZeroPrefix(millis % 1000, 3)
-    }
-
-    // Function to break down time (milliseconds) to hours, minutes and seconds
-    function millisToTimeString(millis, showMilliseconds, showHours) {
-        var hours, minutes, seconds
-
-        // Break down total time (milliseconds) to hours, minutes and seconds
-        seconds = Math.floor(millis / 1000) % 60;
-        minutes = Math.floor(millis / 1000 / 60) % 60
-        hours = Math.floor(millis / 1000 / 60 / 60)
-
-        // Build the time string
-        var timeString = ""
-
-        if (showHours) {
-            timeString += addZeroPrefix(hours, 2) + ":"
-        }
-
-        timeString += addZeroPrefix(minutes, 2) + ":"
-        timeString += addZeroPrefix(seconds, 2)
-
-        if (showMilliseconds) {
-            timeString += "." + millisToString(millis)
-        }
-
-        return timeString
-    }
-
-    // Function to add zero prefix if necessary.
-    function addZeroPrefix (str, totalLength) {
-        return ("00000" + str).slice(-totalLength)
-    }
-
-    // Function to return lap time as a string
-    function lapTimeToString(millis) {
-        var hours = hours = Math.floor(millis / 1000 / 60 / 60)
-        if (hours > 0) {
-            return millisToTimeString(millis, false, true)
-        } else {
-            return millisToTimeString(millis, false, false)
-        }
-    }
-}

=== modified file 'backend/CMakeLists.txt'
--- backend/CMakeLists.txt	2015-08-24 15:08:21 +0000
+++ backend/CMakeLists.txt	2015-08-26 22:51:08 +0000
@@ -41,9 +41,10 @@
 )
 
 set(
-    stopwatchlaphistory_SRCS
-    modules/Stopwatch/LapHistory/backend.cpp
-    modules/Stopwatch/LapHistory/history.cpp
+    stopwatch_SRCS
+    modules/Stopwatch/backend.cpp
+    modules/Stopwatch/history.cpp
+    modules/Stopwatch/formattime.cpp
 )
 
 add_library(timezone MODULE
@@ -66,8 +67,8 @@
     ${clockutility_SRCS}
 )
 
-add_library(stopwatchlaphistory MODULE
-    ${stopwatchlaphistory_SRCS}
+add_library(stopwatch MODULE
+    ${stopwatch_SRCS}
 )
 
 set_target_properties(timezone PROPERTIES
@@ -90,8 +91,8 @@
     LIBRARY_OUTPUT_DIRECTORY Clock/Utility
 )
 
-set_target_properties(stopwatchlaphistory PROPERTIES
-    LIBRARY_OUTPUT_DIRECTORY Stopwatch/LapHistory
+set_target_properties(stopwatch PROPERTIES
+    LIBRARY_OUTPUT_DIRECTORY Stopwatch/
 )
 
 qt5_use_modules(datetime Gui Qml Quick)
@@ -99,7 +100,7 @@
 qt5_use_modules(alarmsettings Gui Qml Quick DBus)
 qt5_use_modules(geolocation Gui Qml Quick)
 qt5_use_modules(clockutility Qml)
-qt5_use_modules(stopwatchlaphistory Qml)
+qt5_use_modules(stopwatch Qml)
 
 # Copy qmldir file to build dir for running in QtCreator
 add_custom_target(timezone-qmldir ALL
@@ -127,8 +128,8 @@
     DEPENDS ${QMLFILES}
 )
 
-add_custom_target(stopwatchlaphistory-qmldir ALL
-    COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/modules/Stopwatch/LapHistory/qmldir ${CMAKE_CURRENT_BINARY_DIR}/Stopwatch/LapHistory
+add_custom_target(stopwatch-qmldir ALL
+    COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/modules/Stopwatch/qmldir ${CMAKE_CURRENT_BINARY_DIR}/Stopwatch
     DEPENDS ${QMLFILES}
 )
 
@@ -148,6 +149,6 @@
 install(TARGETS clockutility DESTINATION ${MODULE_PATH}/Clock/Utility/)
 install(FILES   modules/Clock/Utility/qmldir DESTINATION ${MODULE_PATH}/Clock/Utility/)
 
-install(TARGETS stopwatchlaphistory DESTINATION ${MODULE_PATH}/Stopwatch/LapHistory/)
-install(FILES   modules/Stopwatch/LapHistory/qmldir DESTINATION ${MODULE_PATH}/Stopwatch/LapHistory/)
+install(TARGETS stopwatch DESTINATION ${MODULE_PATH}/Stopwatch/)
+install(FILES   modules/Stopwatch/qmldir DESTINATION ${MODULE_PATH}/Stopwatch/)
 

=== removed directory 'backend/modules/Stopwatch/LapHistory'
=== renamed file 'backend/modules/Stopwatch/LapHistory/backend.cpp' => 'backend/modules/Stopwatch/backend.cpp'
--- backend/modules/Stopwatch/LapHistory/backend.cpp	2015-08-18 05:03:59 +0000
+++ backend/modules/Stopwatch/backend.cpp	2015-08-26 22:51:08 +0000
@@ -20,12 +20,14 @@
 #include <QtQml/QQmlContext>
 #include "backend.h"
 #include "history.h"
+#include "formattime.h"
 
 void BackendPlugin::registerTypes(const char *uri)
 {
-    Q_ASSERT(uri == QLatin1String("Stopwatch.LapHistory"));
+    Q_ASSERT(uri == QLatin1String("Stopwatch"));
 
     qmlRegisterType<LapHistory>(uri, 1, 0, "LapHistory");
+    qmlRegisterType<FormatTime>(uri, 1, 0, "StopwatchFormatTime");
 }
 
 void BackendPlugin::initializeEngine(QQmlEngine *engine, const char *uri)

=== renamed file 'backend/modules/Stopwatch/LapHistory/backend.h' => 'backend/modules/Stopwatch/backend.h'
=== added file 'backend/modules/Stopwatch/formattime.cpp'
--- backend/modules/Stopwatch/formattime.cpp	1970-01-01 00:00:00 +0000
+++ backend/modules/Stopwatch/formattime.cpp	2015-08-26 22:51:08 +0000
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This file is part of Ubuntu Clock App
+ *
+ * Ubuntu Clock App is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Ubuntu Clock App is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "formattime.h"
+
+#include <QtMath>
+
+FormatTime::FormatTime(QObject *parent):
+    QObject(parent)
+{
+}
+
+QString FormatTime::millisToString(int millis) const
+{
+    return addZeroPrefix(QString::number(millis), 3);
+}
+
+QString FormatTime::millisToTimeString(int millis, bool showHours) const
+{
+    int hours, minutes, seconds;
+
+    seconds = qFloor(millis / 1000) % 60;
+    minutes = qFloor(millis / 1000 / 60) % 60;
+    hours = qFloor(millis / 1000 / 60 / 60);
+
+    QString timeString("");
+
+    if (showHours)
+    {
+        if (hours < 10)
+        {
+            timeString += addZeroPrefix(QString::number(hours), 2) + ":";
+        }
+
+        else {
+            timeString += QString::number(hours) + ":";
+        }
+    }
+
+    timeString += addZeroPrefix(QString::number(minutes), 2) + ":";
+    timeString += addZeroPrefix(QString::number(seconds), 2);
+
+    return timeString;
+}
+
+QString FormatTime::addZeroPrefix(QString str, int totalLength) const
+{
+    return QString("00000" + str).remove(0, 5 + str.length() - totalLength);
+}
+
+QString FormatTime::lapTimeToString(int millis) const
+{
+    int hours = qFloor(millis / 1000 / 60 / 60);
+
+    if (hours > 0)
+    {
+        return millisToTimeString(millis, true);
+    }
+
+    else {
+        return millisToTimeString(millis, false);
+    }
+}

=== added file 'backend/modules/Stopwatch/formattime.h'
--- backend/modules/Stopwatch/formattime.h	1970-01-01 00:00:00 +0000
+++ backend/modules/Stopwatch/formattime.h	2015-08-26 22:51:08 +0000
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This file is part of Ubuntu Clock App
+ *
+ * Ubuntu Clock App is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Ubuntu Clock App is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef FORMATTIME_H
+#define FORMATTIME_H
+
+#include <QObject>
+
+class FormatTime: public QObject
+{
+    Q_OBJECT
+
+public:
+    FormatTime(QObject *parent=0);
+
+public slots:
+    QString millisToString(int millis) const;
+    QString millisToTimeString(int millis, bool showHours) const;
+    QString addZeroPrefix(QString str, int totalLength) const;
+    QString lapTimeToString(int millis) const;
+};
+
+#endif

=== renamed file 'backend/modules/Stopwatch/LapHistory/history.cpp' => 'backend/modules/Stopwatch/history.cpp'
=== renamed file 'backend/modules/Stopwatch/LapHistory/history.h' => 'backend/modules/Stopwatch/history.h'
=== renamed file 'backend/modules/Stopwatch/LapHistory/qmldir' => 'backend/modules/Stopwatch/qmldir'
--- backend/modules/Stopwatch/LapHistory/qmldir	2015-08-18 05:03:59 +0000
+++ backend/modules/Stopwatch/qmldir	2015-08-26 22:51:08 +0000
@@ -1,2 +1,2 @@
-module Stopwatch.LapHistory
-plugin stopwatchlaphistory
+module Stopwatch
+plugin stopwatch

=== modified file 'tests/unit/tst_stopwatchUtils.qml'
--- tests/unit/tst_stopwatchUtils.qml	2015-08-17 20:27:12 +0000
+++ tests/unit/tst_stopwatchUtils.qml	2015-08-26 22:51:08 +0000
@@ -18,15 +18,16 @@
 
 import QtQuick 2.4
 import QtTest 1.0
+import Stopwatch 1.0
 import Ubuntu.Components 1.2
 import "../../app/stopwatch"
 
 TestCase {
-    id: stopwatchUtilsTest
-    name: "StopwatchUtilsLibrary"
+    id: stopwatchFormatTimeTest
+    name: "StopwatchFormatTimeLibrary"
 
-    StopwatchUtils {
-        id: stopwatchUtils
+    StopwatchFormatTime {
+        id: stopwatchFormatTime
     }
 
     /*
@@ -35,9 +36,9 @@
     */
     function test_returnMillisecond() {
         var result
-        result = stopwatchUtils.millisToString(400)
+        result = stopwatchFormatTime.millisToString(400)
         compare(result, "400", "Milliseconds not properly converted to the format required")
-        result = stopwatchUtils.millisToString(4)
+        result = stopwatchFormatTime.millisToString(4)
         compare(result, "004", "Milliseconds not properly converted to the format required")
     }
 
@@ -46,9 +47,11 @@
      correctly.
     */
     function test_convertTimeInMillisecondsToString() {
-        var timeInMilliseconds = 1123000
-        var result = stopwatchUtils.millisToTimeString(timeInMilliseconds, false, true)
+        var timeInMilliseconds = 1123000, result
+        result = stopwatchFormatTime.millisToTimeString(timeInMilliseconds, true)
         compare(result, "00:18:43", "Time not properly converted from milliseconds to hh:mm:ss")
+        result = stopwatchFormatTime.millisToTimeString(timeInMilliseconds, false)
+        compare(result, "18:43", "Time not properly converted from milliseconds to mm:ss")
     }
 
     /*
@@ -57,11 +60,11 @@
     */
     function test_zeroPrefixAddedCorrectly() {
         var str = "32", result
-        result = stopwatchUtils.addZeroPrefix(str, 2)
+        result = stopwatchFormatTime.addZeroPrefix(str, 2)
         compare(result, "32", "Zero prefix not added correctly")
-        result = stopwatchUtils.addZeroPrefix(str, 3)
+        result = stopwatchFormatTime.addZeroPrefix(str, 3)
         compare(result, "032", "Zero prefix not added correctly")
-        result = stopwatchUtils.addZeroPrefix(str, 4)
+        result = stopwatchFormatTime.addZeroPrefix(str, 4)
         compare(result, "0032", "Zero prefix not added correctly")
     }
 
@@ -71,9 +74,9 @@
     */
     function test_lapTimeIncludesHoursCorrectly() {
         var result
-        result = stopwatchUtils.lapTimeToString(1123000)
+        result = stopwatchFormatTime.lapTimeToString(1123000)
         compare(result, "18:43", "Lap time shows hours despite it not being greater than 0")
-        result = stopwatchUtils.lapTimeToString(8323000)
+        result = stopwatchFormatTime.lapTimeToString(8323000)
         compare(result, "02:18:43", "Lap time not showing hours despite it being greater than 0")
     }
 }