Frogger-api-task #532
Frogger-api-task #532
Conversation
|
Hey! Congratulations on your PR! 😎😎😎 Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code. Go through the requirements/most common mistakes linked below and fix the code as appropriate. If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat. When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly. Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒 Please, make sure that your code follows the requirements Universal recommendations:
By the way, you may proceed to the next task before this one is reviewed and merged. Sincerely yours, |
|
I've completed self-checks, did'nt find any common errors |
OleksiyRudenko
added
the
self-check-done
There was a problem hiding this comment.
@Conversee12 good job. Although further improvements are required.
The comments below refer items from the requirements
| if (this.x < player.x + 80 && this.y < player.y + 60 && this.x + 80 > player.x && this.y + 60 > player.y) { | ||
| setTimeout(function () { | ||
| player.x = 202; | ||
| player.y = 400; |
There was a problem hiding this comment.
Item 4.iii of the requirements.
Also item 3 (all subitems) of the requirements.
| }; | ||
|
|
||
| let allEnemies = []; | ||
| let eachEnemies = [56, 140, 223]; |
There was a problem hiding this comment.
Item 4 (all subitems) of the requirements
| player.x = 202; | ||
| player.y = 400; |
There was a problem hiding this comment.
Item 4.iii of the requirements. Also why refer to class properties via class instance in the context of class definition?
Also item 3 (all subitems) of the requirements.
Also repetition (item 2 of the requirements).
|
I hope i understood correctly this changes |
There was a problem hiding this comment.
@Conversee12
Feel free asking questions if anything is not clear
| if (this.x < player.x + PlayerWidth && this.y < player.y + PlayerHeight && this.x + PlayerWidth > player.x && this.y + PlayerHeight > player.y) { | ||
| setTimeout(function () { | ||
| player.x = InitialPosition_X; | ||
| player.y = InitialPosition_Y; |
There was a problem hiding this comment.
player is a global variable. 4.iii provides a hint on how to fix the issue.
|
|
Original requirement referred specifically to classes definitions, but yes, writing clean functions (functions that use only what is passed to them explicitly via parameters) is a good practice. What blocks you to re-use your experience with global You do not need to fix this. |
Check what makes arrow functions different from regular functions. |
Something went wrong, I'll try again to figure it out myself |
Read an article about this; understood and corrected |
frogger-api-task
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.