Conversation
|
I found a similar presentation issue with the giveaway.textContent format. If minutes value is 0, it will show '0' instead of '00'. I propose that the issues I pointed out be fixed and then 24 hour time format (without am/pm) is used or add code to convert the default 24 hr time format to 12 hour time with checks do display 12:00AM if Hours == 0. I am relying on the constant variables Hours and Minutes to do this in my fix proposal. Your pull request would invalidate my code. If format() is moved out of getRemaindingTime(), I think it would be better to pass Hours and Minutes to it via function call of format(), since it is a function, rather than alter the current constant variables by sticking format() in there. Assign the format calls to new variables (if necessary) rather than altering the constant variables my fixes rely on for a similar issue elsewhere in the code. That's one of the reasons for declaring something as a constant. So that the constant is immutable, but you can make a copy of it and change and work with the copy. In other words, assign the constant to a variable and alter that variable for the purposes of your piece of the puzzle, and therefore not break other parts of code where others might be relying on the constant to be a constant value in their piece of the puzzle. If constants are frequently needing to be changed at the assignment level, they should not have been declared as a constant to begin with because they effectively aren't "constant" so a var should have been used instead. In very small projects with a small handful of maintainers and excellent communication, or your own code, there is no real impact. But could you imagine a very large project where there are teams of people working on different parts of the code, and the constants are always being altered as in your PR? It would be a continual, and "constant" mess of bugs... (pun intended) and break other parts of the code people are working on. ex: see #61 for clarification on what bugs I'm referring to in project 12 - Countdown - final/app.js |
At the moment, const hours and const minutes haven't been reformat to the desired format.
(For example, if the value is 0, it will show '0' instead of '00')
If we move format() out of getRemaindingTime(), it can be used on const hours and const minutes to get the job done.