-
Notifications
You must be signed in to change notification settings - Fork 255
Conversation
yash-kapila
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Ann-Grabetski - I've finished reviewing your homework. Most of it looks fine but I have a few comments which would be good for you to implement. Also, for week3, I couldn't find submissions for questions 12-17 here https://github.com/HackYourFuture/JavaScript1/blob/master/Week3/MAKEME.md. Could you please have a look? Please drop a message on Slack if you have any questions. Have a good weekend and make sure you use this free week for revising all topics learnt so far. 🙂
| const reverseArray = splitString.reverse(); | ||
| console.log('reverse array: '+reverseArray); | ||
| const joinArray = reverseArray.join(''); | ||
| console.log('corrected string is: '+joinArray); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very well. The logic is correct but you can also reduce it in a single statement using a programming practice called method chaining. For example,
const str = 'dlroW olleH';
const reversedString = str.split('').reverse().join('');
Here I have merged all three operations in a single statement using method chaining.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! It's a very great way! Thank you to show me.
| const reverseArray = splitString.reverse(); | ||
| console.log('reverse array: '+reverseArray); | ||
| const joinArray = reverseArray.join(''); | ||
| console.log('corrected string is: '+joinArray); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering why did you have to repeat this homework submission here? I remember that you were unable to make it to the Week1 where we covered Gitflow in a bit more detail. Let me or Tjebbe know please if you have any confusions or questions around working with Git and we can perhaps work on something. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I did it accidentally then I figured out how to correctly use all this git system.
| const myString = "hello,this,is,a,difficult,to,read,sentence"; | ||
| console.log('my string is: '+myString); | ||
| console.log('length of the string is: '+myString.length+' characters'); | ||
| console.log('corrected string is: '+myString.replace(/,/g,' ')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for logic. I would also like to introduce a new feature in JavaScript ES6 called template literals. It can be found here - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
Using template literals allows you to combine variables and strings and avoid using + operator to concatenate strings.
`corrected string is: ${myString.replace(/,/g, ' '});`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already start to use them. And I love it!
| const reverseArray = splitString.reverse(); | ||
| console.log('reverse array: '+reverseArray); | ||
| const joinArray = reverseArray.join(''); | ||
| console.log('corrected string is: '+joinArray); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This again? 🤔
| console.log ( greetings + ' // ' + language) | ||
| }; | ||
| helloWorld ('Halo, dunia!', 'Indonesian'); | ||
| helloWorld ('Ciao, mondo!', 'Italian'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice of you to create a function for this purpose. 👏
However, there is a slight indentation problem on line 3. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra tab?
| console.log('The actual values of the parameters is:'); | ||
| let x; | ||
| for (x in person) { | ||
| console.log(person[x]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I think a better way to do this would be like this:
function showPropValue(obj) {
for (const prop in obj) {
console.log(obj[prop]);
}
}
showPropValue(person);
What do you think? Can you see some advantages of writing a function like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think I wrote pretty the same function. The only difference is that my function shows not only values but names of properties also. As it was asked in a task.
| console.log('a '+color+' motorbike'); | ||
| }; | ||
| }; | ||
| vehicleType("blue",2); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| @@ -0,0 +1 @@ | |||
| console.log( 3 === 3 ? 'yes' : 'no'); No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| function vehicle(color, code, age) { | ||
| const valueOfColor = color; | ||
| const valueOfCode = vehicleCodes[code]; | ||
| const valueOfAge = age <= 1 ? 'new' : 'used'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good use of conditional operator here 👍
Week3/task08Vehicles.js
Outdated
| let arr1 = vehicleCodes.slice(0, [vehicleCodes.length-1]); | ||
| let str1 = arr1.toString(); | ||
| let str2 = str1.replace(/,/g,', '); | ||
| console.log("Amazing Joe's Garage, we service "+str2+' and '+strLast+'.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I think this isn't complete. The requirement was to have the output like "Amazing Joe's Garage, we service cars, motorbikes, caravans and bikes." with plurals instead of singular values. Could you please have a look again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it wasn't completed. I finished and applied it now.
|
Ann-Grabetski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yash-kapila !
Yes, I didn't finish all of the tasks then. Now I added the missing, and also made implements according to your comments. Please make a re-review. Thank you for your time!
| const reverseArray = splitString.reverse(); | ||
| console.log('reverse array: '+reverseArray); | ||
| const joinArray = reverseArray.join(''); | ||
| console.log('corrected string is: '+joinArray); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! It's a very great way! Thank you to show me.
| const reverseArray = splitString.reverse(); | ||
| console.log('reverse array: '+reverseArray); | ||
| const joinArray = reverseArray.join(''); | ||
| console.log('corrected string is: '+joinArray); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I did it accidentally then I figured out how to correctly use all this git system.
| const myString = "hello,this,is,a,difficult,to,read,sentence"; | ||
| console.log('my string is: '+myString); | ||
| console.log('length of the string is: '+myString.length+' characters'); | ||
| console.log('corrected string is: '+myString.replace(/,/g,' ')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already start to use them. And I love it!
| console.log ( greetings + ' // ' + language) | ||
| }; | ||
| helloWorld ('Halo, dunia!', 'Indonesian'); | ||
| helloWorld ('Ciao, mondo!', 'Italian'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra tab?
| let x; | ||
| console.log("the value of my variable x will be: 13"); | ||
| console.log(x); | ||
| x=13; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it.
| console.log("I think this line is very strange. I don't know why am I writing it :("); | ||
| console.log('The array has a length of: '+favoriteAnimals.length); | ||
| function deleteGiraffe() { | ||
| let giraffe = favoriteAnimals.indexOf("giraffe"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| x = a + b + c; | ||
| return x; | ||
| }; | ||
| abcSum(1,2,3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I understand. Thank you.
| @@ -0,0 +1,14 @@ | |||
| let person = { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| console.log('The actual values of the parameters is:'); | ||
| let x; | ||
| for (x in person) { | ||
| console.log(person[x]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think I wrote pretty the same function. The only difference is that my function shows not only values but names of properties also. As it was asked in a task.
Week3/task08Vehicles.js
Outdated
| let arr1 = vehicleCodes.slice(0, [vehicleCodes.length-1]); | ||
| let str1 = arr1.toString(); | ||
| let str2 = str1.replace(/,/g,', '); | ||
| console.log("Amazing Joe's Garage, we service "+str2+' and '+strLast+'.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it wasn't completed. I finished and applied it now.
Review