branch-bandit / gps-tracker Goto Github PK

1. Lines 2-5 and 11-13,132: you are repeating the initialization of variables. I'd add a initializeState() or setVariablesToDefaultValues() function instead, and I'd call it every time user click START or STOP.
2. Can you not reuse recordCoords() in recordCoordsIfMoved()?
3. Line 60, 61 - these should be consts.
4. onGeoFailure() - do we need isTracking check?
5. Line 100 - ${parsedTime} can be just parsedTime.
6. I would rewrite stopTracking() to

getLocationWithPromise()
	.then((position) => {
		recordCoords(position);
		getResults();
		resetVariables()
	})
	.catch(onGeoFailure);

because it eliminates questions like 'does getResults() accept some variable returned from the recordCoords() promise'.
7. I also thing getResults() doesn't do anything getResulty, I'd just put its contents inline in stopTracking().
8. You don't need the getLocation() function, you can just use getLocationWithPromise()

1. I'd probably rename triggerCalculator() to startOrStopTracking().
2. Instead of current contents of triggerCalculator() I'd have two functions inside: startTracking() and stopTracking(). (showResults() would be named stopTracking() then, because this function's functionality includes a lot more than showing results).
3. Line 166: this line should be moved to the line 171, we don't need it for stopTracking().
4. minDistance never gets reassigned, it's a constant, should be declared with const.
5. I'd delete dynamicTimer and have two functions: startTimer() and stopTimer() instead.
6. updateDynamicTimer and incrementDynamicTime don't need to be global variables.
7. Is there some reason to use default arguments here e.g. updateDynamicTimer = (callback = incrementDynamicTime) => {? You can write it as:

  updateDynamicTimer = () => {
    document.getElementById("time-results").innerHTML = incrementDynamicTime(parseInt(secondsCounted/3600,10)) + ":" + incrementDynamicTime(parseInt(secondsCounted/60,10)) + ":" + incrementDynamicTime(++secondsCounted%60);
  }

8. Same goes for line 75 (position, arr = recordedCoords). Just delete second argument and use recordedCoords[0].
9. Line 118 does nothing.