NW6 | Rabia Avci | JS1 Module | [TECH ED] Complete week 4 exercises | Week 4 #177
Conversation
percentage-change time-format
done is-valid-triangle
done get-card-value
| } | ||
| if (count === 16) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Good work, like the comments as well
Just a small comment (not a telling off or something to be fixed) Loops can be quite bad performance wise and this code isn't the easiest to read, so in the real world I'd be looking hard for shortcuts when I came to refactor. One that springs to mind is
let badCardNum = cardNum[0] * 16;
if (cardNum === badCardNum) {
return false;
| } | ||
| if (total < 16) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Works perfectly and appreciate the comments here, afraid I have two of my own.
- Don't use the same varible for index - it causes bugs something like this is better
while (let index ....)This way each loop manages it's own index. - There is a simpler way of doing this - genrally speaking when there's something like this that is often done, there will be (you'll develop a nose for this eventually)
I would use .split() to make the string into an array https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split and then call .reduce() on the array https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce
| const currentOutput2 = isValidCardNumber(num2); | ||
|
|
||
| //THEN | ||
| expect(currentOutput1).toBe(targetOutput); |
There was a problem hiding this comment.
Nice tests - overall good work needs a little refactoring I think
| let count = 0; | ||
| let index = 0; | ||
|
|
||
| while (index < str.length){ |
There was a problem hiding this comment.
Good work. I do want to see you stopping defining index outside of the loop though. I would also have a look at diffrent types of loops - a for ... in loop would be a better and easier one to use here. https://buzzcoder.gitbooks.io/codecraft-javascript/content/string/loop-through-a-string.html
|
|
||
| test("works for any other number", function () { | ||
| expect(getOrdinalNumber(7)).toBe("7th"); | ||
| expect(getOrdinalNumber(99)).toBe("99th"); |
| // d) What is the condition index < str.length used for? | ||
| // to define when to stop checking characters |
| return `${"Straight angle"}`; | ||
| } | ||
| else if (angle>180 && angle<360){ | ||
| return `${"Reflex angle"}`; |
There was a problem hiding this comment.
All correct. I would avoid using backticks here; they aren't needed, you can just return a string
| return 10; | ||
| } | ||
| throw Error("Invalid card rank."); | ||
| } |
There was a problem hiding this comment.
Simple effective solution with excellent testing 👍
| if (numerator < denominator) { | ||
| return true; | ||
| } else if (numerator > denominator || numerator === denominator) { | ||
| return false; |
| if (a <= 0 || b <= 0 || c <= 0) { | ||
| return false; | ||
| } | ||
| if (a + b <= c || a + c <= b || b + c <= a) { |
Learners, PR Template
Self checklist
Changelist
Rewrite section (as written in Read me) is done, and get used to writing tests.
Implement section completed (except password-validator).
Questions