Skip to content

Conversation

@tylerjhevia
Copy link

Here is an overview of my proposed changes:

  • Refactor API call function:
    - Removed the Promise instantiation in getPersonList function, instead returned fetch directly,
    which already returns a promise
    - Used a catch instead of an if statement to handle unsuccessful fetch
  • Changed getFirstName from arrow function to traditional function. This didn't have any effect on
    functionality, but I wanted to write getFirstName and getLastName in the same style. They do
    serve similar purposes and I think it makes sense to structure them the same way. Is it acceptable
    to use arrow functions and normal functions alongside one another, or should I try pick one syntax
    for a project and stick to it?
  • Altered the shuffleList function so that the result variable is a complete copy of the original list.
    The original copy method with slice(1) excluded the first element in the list array, so I used
    Array.from() instead.
  • Also added ES6 syntax to shuffleList when swapping item positions.
  • The sortByLastName function was calling sortObjListByProp with firstName as the argument and then calling the reverse() method for some reason, so I called sortObjListByProp with lastName as the argument instead.
  • I poked at the React view portion of the file for a while, but it was using different React syntax from what I'm accustomed to, so I didn't end up finding anything to improve. I experimented with changing the syntax of App from React.createClass to App extends React.Component, but I ran into some errors and I wasn't sure that the change would even be an improvement, so I decided to leave it as is.

	- Refactor API call function -- instead of
	instantiating a new Promise, just return fetch
	call, which will return a promise by default
	- Change error handling from if statement to
	catch method
	-Change getFirstName from arrow function
	to traditional function for the sake of
	consistency. There is no significant performance
	benefit to be gained from using arrow functions
	(to my knowledge), and since getFirstname and
	getLastName are performing similar tasks, I want
	to structure them consistently to avoid confusion
	- Replace slice with Array.from
	- List.slice(1) wasn't making a copy of
	the whole list, because it was starting
	at the second value in the array. Array.from
	makes a complete copy
	- Use ES6 syntax to shuffle values
	- sortByLast name was being called with
	firstName as the argument of sortObjListByProp.
	Change firstName to lastName and remove the
	reverse() method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant