Here are 5 JavaScript exercisms, with each exercism starting with a link to my submission. In addition, 5 other solutions for each exercism are reviewed as well, with links and my comments.
My code: link to submission
-
Other Solution #1 (link to submission) - This solution has too many if/else statements and in some cases there is no need for the else clause. I have not not used a single if/else statement since 'return -something- == somethingElse;' notation will return only if it evaluates to true.
-
Other Solution #2 (link to submission) - I follow the logic of the author such as starting with the most narrow conditional and eventually moving towards broader ones. But again, too many if/else statements.
-
Other Solution #3 (link to submission) - My main concern here is defining public methods versus prototype approach. Prototype approach has two benefits that I know of: first, changing the method down the road will apply to all instances of the Leap class. Second, there is efficienty benefit because unless prototype approach is used, each method has to be created every time the constuction function runs.
-
Other Solution #4 (link to submission) - No need for if/else statement and prototype should be used.
-
Other Solution #5 (link to submission) - Iniitally, I thought the conditionals were not accounting for one case, but then I realized two conditionals were combined as (100 & 4 versus 400), interesting approach.
My code: link to submission
-
Other Solution #1 (link to submission) - Great job adopting the prototype approach here. I would use !== instead of != in the conditional becuase it is a good practice and safeguards against some bugs in case of boolean values.
-
Other Solution #2 (link to submission) - Prototype should be used here and no need for the if/else statement, especially the if clause, because it is not changing the sum anyway.
-
Other Solution #3 (link to submission) - I like that returing an error string has been delegated to a method here. However, I would not split the string into arrays because we conveniently use charAt() method iterate over the string characters.
-
Other Solution #4 (link to submission) - We should either initialize the constructor with two strings or we should just pass them when we call the function, but not do both. I would also shortcuts such as diff++.
-
Other Solution #5 (link to submission) - Beautiful implementation here, I would not change anything, except maybe creating better variable names.
My code: link to submission
-
Other Solution #1 (link to submission) - Good job using prototype here. I think forEach is a better choice than my for loop because it is more readable and less code. Both the author and me should have place the converter hash inside the relevant function because we do not want unnecessary global variables.
-
Other Solution #2 (link to submission) - Great solution here. Looks like I did not have to split the string into array and just work with it!
-
Other Solution #3 (link to submission) - If/else statement does not look great and I would use a hash.
-
Other Solution #4 (link to submission) - Another great solution here - I would not change anything here except maybe add couple semicolons here and there.
-
Other Solution #5 (link to submission) - The switch/case statement looks bloated and I suggest using a hash instead.
My code: link to submission
-
Other Solution #1 (link to submission) - Good job defining the function on the prototype. There is no need to divide and multiply by 1000 I believe. That could have been fixed by using Math.pow(10, 12) instead of Math.pow(10, 9).
-
Other Solution #2 (link to submission) - Looks great. A minor change would be to make 1000000000000 more readable in the form of either 1e12 or Math.pow(10,12).
-
Other Solution #3 (link to submission) - I would avoid creating unnecessary variables as in 'var startDate = this.startDate;'.
-
Other Solution #4 (link to submission) - No need for '1e9 x 1000' here, 1e12 will do it.
-
Other Solution #5 (link to submission) - I would not create addedTime method in the constructor for performace reasons.
My code: link to submission
-
Other Solution #1 (link to submission) - Interesting solution here. One character is taken and compared against other characters, pushing the character into the empty array if it is not a match.
-
Other Solution #2 (link to submission) - The function has been built on the constructor instead of prototype. Really good use of a hash to store characters and to check if characters appear more than once. I think hashes allow for better performance as opposed to arrays for tasks of this kind.
-
Other Solution #3 (link to submission) - Good use of an object to store characters again. I also like 'break' inside of for loop which makes the code efficient in that it will not continue iterating once we find 2 of one character.
-
Other Solution #4 (link to submission) - I really like this solution and especially the fact that the author has not spent time on cleaning up the incoming string, which most of us have done with a regex. Instead, he is targeting whatever might come in within the string that is irrelevant.
-
Other Solution #5 (link to submission) - Some great use of slice() here instead of creating a nested loop to iterate over the characters. If it is working ... :)