frogger-game #158
frogger-game #158
Conversation
There was a problem hiding this comment.
@KonstantinOkhorzin good start!
A few improvements are required tho.
| if (this.x > 510) { | ||
| this.x = -50; | ||
| this.speed = Math.round(Math.random() * 100 + 100); | ||
| }; | ||
|
|
||
| if (player.x < this.x + 80 && player.x + 80 > this.x && | ||
| player.y < this.y + 60 && 60 + player.y > this.y) { | ||
| player.x = 200; | ||
| player.y = 400; |
There was a problem hiding this comment.
As a peer developer that needs to contribute to this project I want to understand semantics behind all these magic numbers here and below in the code.
Also apparently update method doesn't just updates position but also has some more of business logic. We need to separate concerns. Please move checks to a different method.
| const enemy1 = new Enemy(-100, 60, 100); | ||
| const enemy2 = new Enemy(-50, 140, 150); | ||
| const enemy3 = new Enemy(-150, 230, 200); |
There was a problem hiding this comment.
As a peer developer on the project I am tasked to add 100 more rows to the field and 100 more enemies. There should be a way to do it programmatically rather than by adding another 100 lines of code.
|
I fixed all. Please, check the changes. |
There was a problem hiding this comment.
This comment remains unadressed: #158 (comment)
What are all these magic numbers?
|
I redid without magic numbers. Please, check the changes. |
There was a problem hiding this comment.
@KonstantinOkhorzin well done! Now the code reads almost in plain English.
Yet one thing to make the code OOP/SOLID compliant.
| if(player.x < this.x + config.canvas.blockWidth && | ||
| player.x + config.canvas.blockWidth > this.x && | ||
| player.y < this.y + config.canvas.blockHeight && | ||
| config.canvas.blockHeight + player.y > this.y) { | ||
| player.x = config.player.initialPositionOnX; | ||
| player.y = config.player.initialPositionOnY; |
There was a problem hiding this comment.
Classes should not refer to any global variables, like global variable player, which is an instance of Player class (referring to global constants and globals provided by the gaming platform like Resources is OK); Hint: pass Player instance as an argument to every enemy
|
I fixed this bug. Please, check the changes. |
Object Oriented JS
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.