Tufahel / test-practice-jest

This is a project-based on JEST implementation in JavaScript. The given tasks are taken from the Microverse exercise.
1 stars 1 forks source link

P2P Review #2

Open damdafayton opened 2 years ago

damdafayton commented 2 years ago

https://github.com/Tufahel/test-practice-jest/blob/1b717b7d42c4d6760eedf4bfef9f595f09b11f66/task1.test.js#L3-L9

Single line of assertion as below would be enough and easier to read: expect(stringLength('tufahel')).toBe(7);

Elerqsousy commented 2 years ago

Amazing job on identifying test cases. I have a couple of suggestions that I think would add to the readability.

  1. Using es6 functions would make your code look much better and cleaner. https://github.com/Tufahel/test-practice-jest/blob/1b717b7d42c4d6760eedf4bfef9f595f09b11f66/task1.test.js#L4 instead of function(){ } you can use () => { }

  2. I believe using unneeded variables would get your code much longer and not clear to some extent. for example, in task 2 https://github.com/Tufahel/test-practice-jest/blob/1b717b7d42c4d6760eedf4bfef9f595f09b11f66/task2.js#L2-L5 you could have summarized these 4 lines in just 2 lines. Especially that you will never use these variables outside this function. like this const reversed = string.split("").reverse().join(""); return reversed;

  3. Using const instead of let if the variable won't get reassigned. https://github.com/Tufahel/test-practice-jest/blob/1b717b7d42c4d6760eedf4bfef9f595f09b11f66/task1.js#L2