-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2048 #544
base: master
Are you sure you want to change the base?
2048 #544
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, left some comments to be reviewed and fixed
src/scripts/main.js
Outdated
} | ||
} | ||
|
||
if (number === 2048) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (number === 2048) { | |
if (number === 2048) { |
Move magic numbers like 2048 to constant
src/scripts/main.js
Outdated
const hasEmptyCell = () => { | ||
for (let r = 0; r < rows; r++) { | ||
for (let c = 0; c < columns; c++) { | ||
if (board[r][c] === 0) { | ||
return true; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use a "some" method here to reduce the amount of code a bit
src/scripts/main.js
Outdated
const slideLeft = () => { | ||
for (let r = 0; r < rows; r++) { | ||
let row = board[r]; | ||
|
||
row = slide(row); | ||
board[r] = row; | ||
|
||
for (let c = 0; c < columns; c++) { | ||
const cell = document.getElementById(`row_${r}-column_${c}`); | ||
const number = board[r][c]; | ||
|
||
updateCell(cell, number); | ||
} | ||
} | ||
|
||
if (hasChanges(initialBoard, board)) { | ||
setCell(); | ||
} | ||
}; | ||
|
||
const slideRight = () => { | ||
for (let r = 0; r < rows; r++) { | ||
let row = board[r]; | ||
|
||
row.reverse(); | ||
row = slide(row); | ||
row.reverse(); | ||
board[r] = row; | ||
|
||
for (let c = 0; c < columns; c++) { | ||
const cell = document.getElementById(`row_${r}-column_${c}`); | ||
const number = board[r][c]; | ||
|
||
updateCell(cell, number); | ||
} | ||
} | ||
|
||
if (hasChanges(initialBoard, board)) { | ||
setCell(); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functions looks similar, i think we can try to combine them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍
Let's improve your code
src/scripts/main.js
Outdated
@@ -1,3 +1,345 @@ | |||
'use strict'; | |||
|
|||
// write your code here | |||
const MAGIC_NUMBER = 2048; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const MAGIC_NUMBER = 2048; | |
const WINNER_SCORE = 2048; |
src/scripts/main.js
Outdated
} | ||
|
||
switch (arrow.key) { | ||
case 'ArrowLeft': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move all these magic words to variables and use it everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work!
DEMO LING