-
-
Notifications
You must be signed in to change notification settings - Fork 41
Week 2 branch #237
base: main
Are you sure you want to change the base?
Week 2 branch #237
Conversation
✅ Deploy Preview for cute-gaufre-e4b4e5 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -1,3 +1,8 @@ | |||
function contains() {} | |||
function contains(obj,key) { |
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.
The function doesn't look quite ready.
For example. Should contains({'a': 1}, 'a')
or contains([1, 2], 1)
return false?
Same. Can you write first on english how the function should work?
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.
obj: should not be null and includes an array
key: should be a string value
If obj is null, return false.
If obj is not an object or an array, return false.
If key is not a string, return false.
If obj is no
contains([1, 2], 1) it returns false because the function takes key as string
in this case contains([1, 2], '1') it returns true
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.
contains([1, 2], '1')
should return true?
It doesn't make any sense, ahah)
1 is not equal to '1'.
I would understand contains(['1', '2'], '1')
returning true.
Is it how the task is given? Or it's your interpretation?
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 Sergey,
Yes you are true It should return false for array input I change the code and I think Its good now
function contains(obj,key) {
if(typeof obj !== "object" || obj === null || Array.isArray(obj)){
return false;
}
return obj.hasOwnProperty(key);
}
test('should return false as an array input', () => {
expect(contains([1, 2], '1')).toBe(false);
});
week-2/implement/lookup.js
Outdated
// implementation here | ||
function createLookup(countryCurrencyPairs) { | ||
const lookUp ={}; | ||
for (const [countryCode,CurrencyCode] of countryCurrencyPairs) { |
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.
Have you noticed that CurrencyCode
has a different color than countryCode
?
It's because CurrencyCode is named as if it would be a class.
Please read https://www.w3schools.com/js/js_conventions.asp.
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 read this thanks
@@ -8,11 +8,11 @@ function totalTill(till) { | |||
let total = 0; | |||
|
|||
for (const [coin, quantity] of Object.entries(till)) { | |||
console.log(coin * quantity); | |||
total += coin * quantity; | |||
// console.log(coin * quantity); |
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.
The function is nicely written.
Could meet something similar in our production code base at work.
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.
Thanks
Complete the challenges of Module-Js2 Week2