Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save philsof/8fbbbdd7e10d168229ba to your computer and use it in GitHub Desktop.
Save philsof/8fbbbdd7e10d168229ba to your computer and use it in GitHub Desktop.
Code review pair-kb-diangelo,kerryimai-javascript-drills.md

Good work! Here are my notes on your code:

  • Nice work creating your own sort function. (Javascript is stupid, it sorts integers as if they are strings. <~~~ FYI this book is the best on JavaScript.)
  • On this line you use ==. Avoid == in JavaScript. Use === instead. This is why.
  • Watch out with ++ and --. This is why.
  • Your countBetween is really solid. Nice.
  • When calculating factorials, it is best to catch 0 and 1 up front (since you already have to write a conditional to catch 0). This is an easy refactor of this line to something like this: if (n <= 1) (Note: the rest of your code will correctly calculate that the factorial of 1 is 1, buy why go through the calculating when you can just add it to the already-existing conditional that catches 0.)
  • This gave a laugh. Not getting to this today!
  • The pad method is a tricky one to write. You successfully figured out how to get it done, which is impressive. FYI Here is another way that destructively pads the array:
    Array.prototype.pad = function(minSize, val) {
    
    if (typeof val === 'undefined'){
      val = null
    };
    
    var leftover = minSize - this.length
    
    if (leftover > 0) {
      for (var i = 0; i < leftover; i++){
        this.push(val);
      };
    };
    
    return this;

};

* Nice [nesting of `for` loops](https://github.com/nyc-chorus-frogs-2016/javascript-drills/blob/041658634552fb3a3d2b36347d04bc89df316b82/times_table.js). Watch your indentation though.
* For the [timesTable](https://github.com/nyc-chorus-frogs-2016/javascript-drills/blob/041658634552fb3a3d2b36347d04bc89df316b82/times_table.js): you are currently returning the entire array, which is why the output doesn't look like you want it to. If you want to make the output look nice, you would need to add line breaks to the end of each line of the times table, and then turn the array into a string, and then print the string. 

Good work! Any questions, let me know.

-Phil
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment