Solving Project 2 Problems by paulghaddad · Pull Request #1 · paulghaddad/Basic-JavaScript

paulghaddad · 2017-09-25T12:10:39Z

No description provided.

chichiwang · 2017-09-25T13:26:55Z

src/project-2.js

 const getBiggest = (x, y) => {
  // x and y are integers.  Return the larger integer
  // if they are the same return either one
+  return x > y ? x : y;


Opportunity for refactor - One-liner:

const getBiggest = (x, y) => x > y ? x : y;

chichiwang · 2017-09-25T13:27:48Z

src/project-2.js

+  }
 };

 const isTenOrFive = (num) => {


Opportunity for refactor - One-liner:

const isTenOrFive = num => num === 10 || num === 5 ? true : false;

👍 And even simpler: const isTenOrFive = num => (num === 10 || num === 5);

chichiwang · 2017-09-25T13:29:57Z

src/project-2.js


 const returnFirst = (arr) => {
  // return the first item from the array
+  return arr.shift();


Opportunity for refactor - mutation. shift mutates the parameter passed in and causes this method to have side effects. When working functionally, avoid all side-effects.

const returnFirst = arr => arr[0];

Great catch!

chichiwang · 2017-09-25T13:30:33Z

src/project-2.js


 const returnLast = (arr) => {
  // return the last item of the array
+  return arr.pop();


Opportunity for refactor - mutation. pop mutates the parameter passed in and causes this method to have side effects. When working functionally, avoid all side-effects.

const returnLast = arr => arr[arr.length - 1];

chichiwang · 2017-09-25T13:31:23Z

src/project-2.js


 const getArrayLength = (arr) => {
  // return the length of the array
+  return arr.length;


Opportunity for refactor - One-liner

const getArrayLength = arr => arr.length;

chichiwang · 2017-09-25T13:33:37Z

src/project-2.js


 const isInRange = (num) => {
  // return true if num is less than 50 and greater than 20
+  if (num < 50 && num > 20) {


Opportunity for refactor - One-liner:

const isInRange = num => num < 50 && num >20 ? true : false;

chichiwang · 2017-09-25T13:35:04Z

src/project-2.js

  // arr is an array of integers
  // increase each integer by one
  // return the array
+  const incrementedArray = arr.map(element => element += 1);


Opportunity for refactor - One-liner

const incrementByOne = arr => arr.map(el => el + 1);

chichiwang · 2017-09-25T13:36:14Z

src/project-2.js

 const addItemToArray = (arr, item) => {
  // add the item to the end of the array
  // return the array
+  arr.push(item);


Opportunity for refactor - mutation. push mutates the parameter passed in and causes this method to have side effects. When working functionally, avoid all side-effects.

const addItemToArray = (arr, item) => [...arr, item];

chichiwang · 2017-09-25T13:36:50Z

src/project-2.js

  // add the item to the front of the array
  // return the array
  // hint: use the array method .unshift
+  arr.unshift(item);


Opportunity for refactor - mutation. unshift mutates the parameter passed in and causes this method to have side effects. When working functionally, avoid all side-effects.

const addItemToFront = (arr, item) => [item, ...arr];

chichiwang · 2017-09-25T13:37:40Z

src/project-2.js

  // spaces need to be between each word
  // example: ['Hello', 'world!'] -> 'Hello world!'
+
+  return words.join(' ');


Opportunity for refactor - One-liner

const wordsToSentence = words => words.join(' ');

chichiwang · 2017-09-25T13:38:29Z

src/project-2.js

 const contains = (arr, item) => {
  // check to see if item is inside of arr
  // return true if it is, otherwise return false
+  const elementPresent = arr.indexOf(item);


Opportunity for refactor - One-liner

const contains = (arr, item) => arr.indexOf(item) >= 0;

Used includes instead:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes

const contains = (arr, item) => arr.includes(item);

includes is fine for the exercise, but we will still be using indexOf for a long time in production (as it is not supported in our favorite browser as of yet).

chichiwang · 2017-09-25T13:39:29Z

src/project-2.js

 const addNumbers = (numbers) => {
  // numbers is an array of integers.
  // add all of the integers and return the value
+  return numbers.reduce((sum, num) => sum + num);


Opportunity for refactor - One-liner

const addNumbers = numbers => numbers.reduce((sum, num) => sum + num);

chichiwang · 2017-09-25T13:40:42Z

src/project-2.js

 const averageTestScore = (testScores) => {
  // testScores is an array.  Iterate over testScores and compute the average.
  // return the average
+  const sum = testScores.reduce((memo, num) => memo + num);


Opportunity for refactor - One-liner

const averageTestScore = testScores => addNumbers(testScores)/testScores.length;

chichiwang · 2017-09-25T13:45:00Z

src/project-2.js

 const largestNumber = (numbers) => {
  // numbers is an array of integers
  // return the largest integer
+  return numbers.reduce((largestItem, num) => {


Opportunity for refactor - One-liner

const largestNumber = numbers.reduce((num1, num2) => Math.max(num1, num2));

chichiwang · 2017-09-25T13:51:18Z

@paulghaddad Very solid work - mostly just take advantage of lambda syntax wherever possible. It's hard to grok at first, but we may as well get used to functional notation.

On a side note, at our shop we've made the decision to skip arrow notation in favor of function notation except where we explicitly want the behavior of arrow notation.

We spent the period of a week sharing resources and a meeting discussing it: https://developer.ibm.com/node/2015/09/21/an-introduction-to-javascript-es6-arrow-functions/

Arrow notation applies a lexical this and binds the context in which it is declared. It is essentially Function.proptotype.bind behavior. We decided we should only use it when we explicitly want that behavior (React Class methods that are event handlers, for instance).

Arrow notations also do not benefit from hoisting. We've seen arguments in favor of and against this behavior.

Also anonymous arrow functions (and anonymous functions in general) leave a more confusing stacktrace.

Benchmarking arrow notation against the function keyword, I found that in browsers across the board, the function keyword outperforms arrow notation.

paulghaddad · 2017-09-25T22:23:10Z

@chichiwang Fantastic review! I learned a lot! Just updated the PR. Thank you so much! :)

chichiwang · 2017-09-25T22:44:27Z

src/project-2.js


-  return arr;
-};
+const incrementByOne = arr => arr.map(element => element += 1);


No need to update element here, as the result is implicitly returned from the arrow function anyway. element + 1 will work and skip the assignment.

paulghaddad added 2 commits September 24, 2017 13:53

Solving Project 2 Problems

9e53bd4

Completion of Project 2 Exercises

0a8e868

chichiwang reviewed Sep 25, 2017

View reviewed changes

Changes per Chi Chi's review

e1c23a8

paulghaddad force-pushed the project-2 branch from 0daf3e8 to e1c23a8 Compare September 26, 2017 11:29

paulghaddad merged commit 90b6964 into master Sep 26, 2017

paulghaddad deleted the project-2 branch September 26, 2017 12:57

Conversation

paulghaddad commented Sep 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chichiwang Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chichiwang Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chichiwang Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chichiwang commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulghaddad commented Sep 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chichiwang Sep 25, 2017 •

edited

Loading

chichiwang Sep 25, 2017 •

edited

Loading

chichiwang Sep 25, 2017 •

edited

Loading

chichiwang commented Sep 25, 2017 •

edited

Loading