NW | 25-ITP_Sep | Ahmad Hmedan | Sprint 3 | implement and rewrite tests #812
Conversation
AhmadHmedann
added
the
Needs Review
| if (numerator < 1) numerator = numerator * -1; | ||
| if (denominator < 1) denominator = Math.abs(denominator); |
There was a problem hiding this comment.
Why use different approaches to remove the negative sign from a number?
There was a problem hiding this comment.
I don't know if you want me to use an existing library or solve it manually.
There was a problem hiding this comment.
In real interview, developers are expected to explain their code and give a rational explanation why they do things in certain way.
Your code looks odd to me, that's why I raised the question.
There was a problem hiding this comment.
Thanks for your feedback. From now on, I’ll try to write my code as if I’m working.
| const convertTheStringtoNumber = Number(rank); | ||
| if (convertTheStringtoNumber >= 2 && convertTheStringtoNumber <= 10) { |
There was a problem hiding this comment.
In JavaScript, strings that represent valid numeric literals in the language can be safely
converted to equivalent numbers or parsed into a valid integers.
Do you want to recognize these string values as valid ranks?
To find out what these strings are, you can ask AI
What kinds of string values would make
Number(rank)evaluate to2in JS?
There was a problem hiding this comment.
Thanks, I’ve learned a new validation technique from your feedback.
| test("returns true for a negative proper fraction( absolute value of the numerator is less than the denominator)", () => { | ||
| expect(isProperFraction(-4, 7)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
When preparing tests, we should ensure the tests cover all possible cases (and maybe test multiple samples within each case to make the test more robust). Can you think of case(s) that should also be tested?
cjyuan
added
Reviewed
AhmadHmedann
added
Needs Review
| if (numerator < 1) numerator = Math.abs(numerator) ; | ||
| if (denominator < 1) denominator = Math.abs(denominator); |
There was a problem hiding this comment.
denominator < 0 is probably more expressive in the sense that it is closer to the definition of "negative number".
| test("returns false for a negative proper fraction( absolute value of the denominator is less than the numerator)", () => { | ||
| expect(isProperFraction(-7, 4)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
- -7/4 is considered a negative improper fraction.
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
I have solved all the tasks in the coursework
Thanks in advance for reviewing my code
Questions
none,