Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

South Africa Class 1 - Jarrod Benjamin - JS-Core 1 - Week 1#36

Closed
JarrodBen wants to merge 5 commits into
CodeYourFuture:masterfrom
JarrodBen:master
Closed

South Africa Class 1 - Jarrod Benjamin - JS-Core 1 - Week 1#36
JarrodBen wants to merge 5 commits into
CodeYourFuture:masterfrom
JarrodBen:master

Conversation

@JarrodBen
Copy link
Copy Markdown

Your Details

  • Your Name: Jarrod
  • Your City: Cape Town - South Africa
  • Your Slack Name: Jarrod Benjamin

Homework Details

  • Module: JSCore 1
  • Week: Week 1

Did up until "I" and am now busy with "J", I will do the rest till "L" tomorrow.
Did the last of the exercises that I had not finished in the 1st commit
Finished off all the 4 exercises in the mandatory folder
Just fixed my answer to this exercise.
@Andile-coder
Copy link
Copy Markdown

Hi Ben, your code is very clean and readable. Looking at exercises/L-functions-nested/exercise2.js you can also have one function which converts the name to upper case and prints the greeting message. I like the way you commit it makes it easy to follow along your code. Keep up the good work.

@JarrodBen
Copy link
Copy Markdown
Author

Hey Andile, I will consider doing that with that exercise, thanks for the feedback, appreciate it. 👍

Comment thread exercises/G-numbers/exercise.js Outdated
@@ -1 +1,10 @@
// Start by creating a variables `numberOfStudents` and `numberOfMentors`
var numberOfStudents = "Number of students: 15"
Copy link
Copy Markdown

@sirpfaira sirpfaira Jan 21, 2021

Choose a reason for hiding this comment

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

I think code should have been like, <<"Number of students: " + numberOfStudents

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Noted, will take care of it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks for the feedback.

Comment thread exercises/I-floats/exercise.js Outdated
var numberOfStudents = 15;
var numberOfMentors = 8;

var quotientStudents = 15/23 * 100
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should not hard code the values of the variables, use the variable names instead. e.g. quotientStudents = numberOfStudents / (numberOfStudents + numberOfMentors) * 100

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Noted, will take care of it.

Copy link
Copy Markdown
Author

@JarrodBen JarrodBen Jan 22, 2021

Choose a reason for hiding this comment

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

Done, thanks for the feedback.

Comment thread mandatory/3-function-output.js Outdated
}

// Add comments to explain what this function does. You're meant to use Google!
// This function is going to concatenate the string arguments to the calling string and then returns the new string
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You lost me. What is a calling string?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The calling string is what comes before the .concat so in this case it would be w1

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks for the feedback.

Comment thread mandatory/3-function-output.js Outdated
@@ -1,16 +1,21 @@
// Add comments to explain what this function does. You're meant to use Google!
// Add comments to explain what this function does. You're meant to use Google!
/* Math.random returns a pseudo-random number between 0 - less than 1 (by default)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This definition is kinda generic. You should have tailored it to what the underlying method do

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will look up for a new more defining definition.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks for the feedback.

Comment thread mandatory/4-tax.js Outdated

function calculateSalesTax() {}
function calculateSalesTax(price) {
const taxChargeAmount = price + (20 / 100) * price
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sometimes you have to avoid declaring too much variables in order to shorten your code. Try putting the "method operation" statement on the return statement if its short enough

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll shorten it right away.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks for the feedback.

Fixed and shortened some code and fixed some comments.
@github-actions
Copy link
Copy Markdown

Your coursework submission has been closed because nobody has interacted with it in 30 days. You are welcome to re-open it to get more feedback.

@github-actions github-actions Bot added the Stale label May 13, 2021
@github-actions github-actions Bot closed this May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants