search

icyflame / cli-cube-timer

Time your solves, without leaving the terminal
https://npmjs.org/package/cli-cube-timer
20 stars 5 forks source link

Use null instead of NaN as default value for solvestats-module.calcStats result #37

Closed Jan-Ka closed 7 years ago

Jan-Ka commented 7 years ago

While working on #30 I found that by default the result of solvestats-module.calcStats will return

{
  ao5: NaN,
  ao12: NaN,
  ao_session: NaN,
  best_time: NaN,
  worst_time: NaN
}

NaN is not a good value to compare against, I propose to change the default value to null. To quote one of my unit-tests (b0194129b7e4f25905880f37a8c7b68ee6968402) why NaN is suboptimal:

// NaN is a bad value for comparison
// however - NaN is the only value that does not compare equal to itself
// you might attempt to employ isNaN()
// but isNaN(NaN) === isNaN("nan")

Edit: Sorry, hit NumPad Enter while reaching for the Mouse :(

icyflame commented 7 years ago

Hey @Jan-Ka , I was finally able to review #39. You are right, NaN is a bad default. -1 is a good default value, you can use that. (We needn't use null because all these are times which can never be less than 0. Also, if something goes wrong and the default value is printed, -1 will be better to read than null)