← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

Re: [Merge] lp:~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-timezone-fix into lp:ubuntu-clock-app

 

Review: Needs Information

Just got some inline code doubts.

Diff comments:

> === modified file 'app/stopwatch/StopwatchPage.qml'
> --- app/stopwatch/StopwatchPage.qml	2015-08-25 01:02:54 +0000
> +++ app/stopwatch/StopwatchPage.qml	2015-09-01 15:24:48 +0000
> @@ -37,28 +37,42 @@
>          console.log("[LOG]: Stopwatch Page Loaded")
>      }
>  

can't we just do,
return new Date(Date.UTC()) // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/UTC

I haven't tested this, just curious if we could simplify the code.

> +    function getUTCDate() {
> +        var localDate = new Date()
> +        return new Date(localDate.getUTCFullYear(),
> +                        localDate.getUTCMonth(),
> +                        localDate.getUTCDate(),
> +                        localDate.getUTCHours(),
> +                        localDate.getUTCMinutes(),
> +                        localDate.getUTCSeconds(),
> +                        localDate.getUTCMilliseconds());
> +    }
> +
>      function start() {
> -        startTime = new Date()
> +        startTime = getUTCDate()
>          snapshot = startTime
>          running = true
>      }
>  
>      function stop() {
>          oldDiff += timeDiff
> -        startTime = new Date()
> +        startTime = getUTCDate()
>          snapshot = startTime
>          running = false
>      }
>  
>      function update() {
> -        snapshot = new Date()
> +        snapshot = getUTCDate()
> +        timeDiff = snapshot - startTime

Why the addition of, timeDiff = snapshot -startTime and totalTimeDiff = timeDiff + oldDiff ? Both these variable have been set at the root of the page and will/should automatically update when you set snapshot = getUTCDate().

> +        totalTimeDiff = timeDiff + oldDiff
>      }
>  
>      function clear() {
>          oldDiff = 0
> -        startTime = new Date()
> +        startTime = getUTCDate()
>          snapshot = startTime
>          lapHistory.clear()
> +        totalTimeDiff = 0

Same question here, why totalTimeDiff = 0 should be explicitly done? totalTimeDiff has been defined to be timeDiff + oldDiff and these variables will automatically change after setting startTime = getUTCDate().

>      }
>  
>      Timer {


-- 
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-stopwatch-timezone-fix/+merge/269768
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.


References