Skip to content
This repository was archived by the owner on Oct 26, 2020. It is now read-only.

Hiba's homework WK1 (Mandatory+Extra) #3

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

Hiba-moh
Copy link

@Hiba-moh Hiba-moh commented Jun 14, 2020

Your Details

Your Name:Hiba Mohammed
Your City:Birminham
Your Slack Name:Hiba

Homework Details

JavaScript-Core-1-Homework
Module:JavaScript
Week:1

Hiba-moh added 22 commits June 14, 2020 04:31
Added the  missing commas between addNumbers function arguments.
introduceMe function
1- missed the curly bracets
2- missed the + to concatenate the sentences
3- some spaces required to make the sentence readable
1- took off the additional (+) sign
2- string interpolation uses backstick instead of double quotes and a dollar sign instead of the percent sign.
created the missing getRemainder
Indented the code and I took the comments out
The function has to return the passed argument wich is word .
The function has to return the length of the passed argument
multiply function returns the multiple of the three arguments.
Trim word should return the text passed into the function without the white space using the method (trim())
indented the code and took the comments off
Added comment explained  getNumber output
Function S takes two strings and concatenate them together
Indented the code and took of the comments
built the formatCurrency function also
built separated fuction to calculate the tax and the function calculateSalesTax(the last price ) wich add the tax to the price
Indentation + comments out
@Hiba-moh Hiba-moh changed the title Hiba's homework Hiba's homework (Mandatory+Extra) Jun 14, 2020
@Hiba-moh Hiba-moh changed the title Hiba's homework (Mandatory+Extra) Hiba's homework WK1 (Mandatory+Extra) Jun 14, 2020
I've added acomment to the bad practice .. I've missed it earlier
Copy link

@Rody-Kirwan Rody-Kirwan left a comment

Choose a reason for hiding this comment

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

Really good work Hiba. Well done.
I left some notes but nothing to worry about. :)

/******************************************
* This function takes two strings and
* join (concatenate) them together
* ****************************************/

Choose a reason for hiding this comment

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

True - but these could also be arrays :)
[1, 2, 3].concat([6, 7, 8]) // returns [1, 2, 3, 6, 7, 8]

function s(w1, w2) {
return w1.concat(w2);
}

function concatenate(firstWord, secondWord, thirdWord) {
// Write the body of this function to concatenate three words together
// Look at the test case below to understand what to expect in return
return firstWord + " ".concat(secondWord + " ", thirdWord);

Choose a reason for hiding this comment

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

Nice. You could also use template strings to get the same result.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals

return `${firstWord} ${secondWord} ${thirdWord}`

*/

function formatCurrency() {}
function sTax(Price) {

Choose a reason for hiding this comment

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

Use lowercase (camelCase) for javascript variables, functions and params.
We only use uppercase for Classes and Components. Also - always give functions readable names that explain what the function does. This is not always easy but try to take the time as it has huge benefits for the readability of your code.

let tax = sTax(Price);
lastPrice = Price + tax;
return "£" + lastPrice.toFixed(2);
}

function test(test_name, expr) {
let status;

Choose a reason for hiding this comment

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

This is well done.

A couple of notes FYI.

Nothing you can do but - one of the tests is badly written in my opinion.
the test for formatCurrency demands that the "calculation of price" should also be included.
This would be better named getFullPriceAsFormattedCurrency as it does much more than simply "formatCurrency".

I would keep actually keep the function as formatCurrency which just accepts a value and returns the same value as a currency.
This allows us to reuse it again and again anytime we need to print a value as currency.
Then to print the fullPrice in currency you would run

formatCurrency( calculateFullPrice(15) )

With that in mind we can simplify this program for the reader.

function calculatePercentage(percent, value) {
  return (percent / 100) * value
}

function calculateSalesTax(price) {
  return calculatePercentage(20, price)
}

function calculateFullPrice(price) {
  return calculateSalesTax(price) + price
}

function formatCurrency(value) {
  return `£${value.toFixed(2)}`
}

Again nothing you did wrong here - just some extra info

@@ -12,30 +12,36 @@
- multiply the result by 2

Choose a reason for hiding this comment

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

aha - I see they have a similar idea :D.
Nice

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.

2 participants