Code review by anywhereiromy · Pull Request #1 · codeandstuff-manchester/javascript-basics

anywhereiromy · 2021-11-30T18:15:07Z

No description provided.

anywhereiromy · 2021-11-30T18:15:23Z

+const array = ['cat', 'dog', 'elephant', 'fox'];
 const getNthElement = (index, array) => {
-  // your code here
+  return array[index % array.length];


anywhereiromy · 2021-11-30T18:17:03Z


 const addToArray = (element, array) => {
  // your code here
+  console.log(array.push(element));


It is good practice to remove console.log() s from your code once you are finished. This is because you usually don't want to display data to users in the console from within your application/programme where possible. It could be sensitive information and it might also create a lot of noise in case there are other important messages or errors in the console.

Just writing array.push(element) here would be great

anywhereiromy · 2021-11-30T18:18:39Z


 const numbersToStrings = numbers => {
  // your code here
+  return numbers.map(String);


This is really clever. +1

anywhereiromy · 2021-11-30T18:20:43Z


 const onlyEven = numbers => {
  // your code here
+  return numbers.filter(x => x % 2 === 0);


This works perfectly, the only suggestion I would have is usually we like to describe what a parameter is through it's naming, so instead of x here, I would probably write number or num. Just because it is slightly more descriptive, as the letter x could represent anything.

anywhereiromy · 2021-11-30T18:21:03Z

 const removeNthElement2 = (index, array) => {
  // your code here
-};
+  const newArray = [...array];


Nice use of the spread operator. This is a really important and commonly used operator.

anywhereiromy · 2021-11-30T18:22:18Z

 const elementsStartingWithAVowel = strings => {
  // your code here
-};
+  return strings.filter(string => (string.match(/^[aeiou]/gi)));


You have one extra set of brackets her that I don't think you need. return strings.filter(string => string.match(/^[aeiou]/gi));

anywhereiromy · 2021-11-30T18:22:33Z


 const removeSpaces = string => {
  // your code here
+  return string.replace(/\s+/g, '');


nice use of replace

anywhereiromy · 2021-11-30T18:22:47Z


 const sumNumbers = numbers => {
  // your code here
+  return numbers.reduce((total, current) => total + current, 0)


nice use of reduce

anywhereiromy · 2021-11-30T18:23:02Z


 const sortByLastLetter = strings => {
  // your code here
+  return strings.sort((x, y) => x.charCodeAt(x.length - 1) - y.charCodeAt(y.length - 1));


nice use of character codes

anywhereiromy · 2021-11-30T18:23:21Z

-function negate(a) {
-  // your code here
-};
+const negate = (a) => !a;


Woo! Implicit returning :)

anywhereiromy · 2021-11-30T18:24:32Z

-  // your code here
-};
+const both = (a, b) => {
+  if (!a || !b) {


You could use return Boolean(a || b) or return !!(a || b) here if you wanted to get rid of the if else statement.

anywhereiromy · 2021-11-30T18:24:45Z

-};
+const either = (a, b) => {
+  if (a || b) {
+    return true;


Same refactor here

anywhereiromy · 2021-11-30T18:26:56Z

+  return false;
+}
+const none = (a, b) => {
+  if (a || b) {


I like this because it is a slightly different solution to what I expected, but it still works. I would use return !a && !b here.

anywhereiromy · 2021-11-30T18:27:57Z

-};
+const truthiness = (a) => {
+  if(a){
+    return true;


You can use Boolean(a) or !!(a) here to get rid of the if else statement

anywhereiromy · 2021-11-30T18:28:49Z

-};
+const one = (a, b) => {
+  if ((a && !b) || (b && !a)) {
+    return true;


You can use return Boolean(((a && !b) || (b && !a))) or return !!((a && !b) || (b && !a)) here to get rid of the if/else statement.

anywhereiromy · 2021-11-30T18:29:22Z

+const isEqual = (a, b) => {
+  if (a === b){
+    return true;
+  }


You can just return a === b here as the comparison logic evaluates to a boolean already. :)

anywhereiromy · 2021-11-30T18:29:40Z

+const isGreaterThan = (a, b) => {
+  if (a > b){
+    return true;
+  }


Same here, return a > b will also work.

anywhereiromy · 2021-11-30T18:31:02Z

-};
+const startsWith = (char, string) => {
+  if (string.startsWith(char) === true){
+    return true;


Same here you can use return string.startsWith(char) as that method already returns true or false

anywhereiromy · 2021-11-30T18:31:42Z

+
+const isLowerCase = (string) => {
+  if (string === string.toLowerCase()){
+    return true;


Your answers are all really good, you can just shorten them by getting rid of the if else statements like I have suggested above :)

anywhereiromy · 2021-11-30T18:32:04Z

-function remainder (a, b) {
-  // your code here
-}
+const remainder = (a, b) => a % b;


Absolutely perfect 👯

anywhereiromy · 2021-11-30T18:32:58Z


 const hasProperty = (property, object) => {
  // your code here
+  return object[property]? true : false;


Nice use of a ternary here! +1 You could also us the hasOwnProperty method https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty

anywhereiromy · 2021-11-30T18:33:19Z


 const isOver65 = person => {
  // your code here
+  return person.age > 65? true : false;


You can just shorten this to person.age > 65

anywhereiromy · 2021-11-30T18:34:22Z


 const getAges = people => {
  // your code here
+  return people.map(ageArray => ageArray.age);


Technically we are mapping over the people array here, so each iteration is using a person. I would change the variable name from ageArray to person to be more accurate.

anywhereiromy · 2021-11-30T18:34:32Z


 const findByName = (name, people) => {
  // your code here
+  return people.find(person => person.name === name);


anywhereiromy · 2021-11-30T18:35:20Z


 const findHondas = cars => {
  // your code here
+  return cars.filter(carModel => carModel.manufacturer === 'Honda');


This is great. I would just suggest naming the variable car instead of carModel to be more precise as the variable actually contains more than just information about the model.

anywhereiromy · 2021-11-30T18:35:27Z


 const averageAge = people => {
  // your code here
+  return people.reduce((a,b) => a + b.age, 0) / people.length;


anywhereiromy · 2021-11-30T18:35:42Z

+  return {
+    name: name,
+    age: age,
+    introduce: stranger => {


anywhereiromy · 2021-11-30T18:36:02Z

@@ -1,41 +1,59 @@
 const createPerson = (name, age) => {
  // your code here
+  this.name = name;


Really good use of this here.

Most people have been writing object literals like { name, age }, but both work.

anywhereiromy · 2021-11-30T18:37:21Z

-function firstCharacter (string) {
-  // your code here
-};
+const firstCharacter = (firstAlp) => firstAlp[0];


I would probably rename firstAlp to be string here as the tests can input any string.

anywhereiromy · 2021-11-30T18:37:35Z

-function firstCharacters (string, n) {
-  // your code here
-};
+const firstCharacters = (string, n) => string.slice(0, n);


Perfect functions :D

anywhereiromy

Really good work! Just a few minor tweaks suggested.

JMoss89 added 8 commits November 24, 2021 08:04

completed strings.js test

7b81267

completed numbers.js test

33625c2

completed booleans.js test

0b71e58

completed arrays.js test

88a8c68

completed objects.js test

7900057

refactored numbers.js

6eafa75

refactored strings.js

d9f9811

refactored booleans.js

8c785c5

anywhereiromy commented Nov 30, 2021

View reviewed changes

Comment thread src/objects.js

return {

name: name,

age: age,

introduce: stranger => {

Copy link
Copy Markdown

Collaborator Author

anywhereiromy Nov 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

anywhereiromy commented Nov 30, 2021

View reviewed changes

Conversation

anywhereiromy commented Nov 30, 2021

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!

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!

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!

anywhereiromy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants