patternjavascriptMinor
MV* Pattern for a board game
Viewed 0 times
gameforpatternboard
Problem
I have made a small game in Javascript, while simultaneously learn about the ever so popular model view controller. The game is this, there are 3 players(green, purple, yellow) and they have to try and build a path of arrows to the blue square. The first player to make the path go to the blue square wins. When the path reaches the top row, the players go back to the start, and redirect the old path.
I have done this with just plain javascript, jQuery, and a little CSS. Ideally I was hoping to use this as a project to learn React.js, and Backbone.js, but I should get it into basic MV* form first.
EDIT:
The previous version of this, was asking how to rewrite my code to fit an MV* pattern. I have done a massive refactorization of the code. It's in an MVC, or at least what I think is an MVC pattern. Is this the best way to arrange the code, or is there another way to better fit the game?
I still indent to bring in React.js and Backbone.js so any additional tips for that would be nice.
JSFiddle for a playable version
CSS:
JS:
```
var UP = 0;
var RIGHT = 45;
var LEFT = 315;
function print(stuff) {
console.log(stuff);
}
//arrow should technically be a part of Display, but it's just too cumbersome to put in with the rest of the code.
function arrow(direction) {
return " ";
}
function Cell(row, column) {
this.row = row;
this.column = column;
this.direction = null;
this.get_row = function () {
return this.row;
};
this.get_column = function () {
return this.column;
I have done this with just plain javascript, jQuery, and a little CSS. Ideally I was hoping to use this as a project to learn React.js, and Backbone.js, but I should get it into basic MV* form first.
EDIT:
The previous version of this, was asking how to rewrite my code to fit an MV* pattern. I have done a massive refactorization of the code. It's in an MVC, or at least what I think is an MVC pattern. Is this the best way to arrange the code, or is there another way to better fit the game?
I still indent to bring in React.js and Backbone.js so any additional tips for that would be nice.
JSFiddle for a playable version
CSS:
table, th, td {
border: 3px solid #ccc;
border-collapse:collapse
}
svg {
display: block;
}
td {
box-sizing: border-box;
width: 33px;
height: 33px;
padding: 0px;
}
.arrow_to_place_container {
display: inline-block;
border: 3px solid #aaa;
}
.win_position {
background-color: blue;
}
.player_1 {
background-color: green;
}
.player_2 {
background-color: purple;
}
.player_3 {
background-color: yellow;
}JS:
```
var UP = 0;
var RIGHT = 45;
var LEFT = 315;
function print(stuff) {
console.log(stuff);
}
//arrow should technically be a part of Display, but it's just too cumbersome to put in with the rest of the code.
function arrow(direction) {
return " ";
}
function Cell(row, column) {
this.row = row;
this.column = column;
this.direction = null;
this.get_row = function () {
return this.row;
};
this.get_column = function () {
return this.column;
Solution
Work In Progress,
interesting code, I had to try a few times to understand how to win the game. Please dont change the code again, since I made a copy and I am working on that copy.
UI
While not typically part of CodeReview, the UI should give you all the hints needed to play the game. The fact that 'R' should be pressed should be mentioned, even more important, this
could have been this:
or even this:
Also, I don't know why the user has to click the colored box, is that to prevent accidental clicks? It was not intuitive to me.
Constants
Style
*
JsHint.com
Objects
It is more idiomatic in JavaScript to forego getters and setters until you actually need them, so this:
Should really only be this:
If you ever need processing within getters and setters, deal with it then. I am willing to bet that you will never need it for this project. I left out
Furthermore, when you wrap an object around an array like you do for
It means some code will be doing things that the Board class should be doing. Fortunately this function is never called and I could just remove it. Read up on YAGNI.
Furthermore, I have some issues with functions like these:
The board has no business knowing the direction of a cell, it knows its own next arrow direction and that should be it. It could be argued that this was done for syntactic sugar but I dont see much of a benefit when I compare these:
vs
On directions
You should read up on DRY, it is a great engineering (and life) principle that you could apply to this code:
You could simply encode the sequence of directions so that
I also renamed
interesting code, I had to try a few times to understand how to win the game. Please dont change the code again, since I made a copy and I am working on that copy.
UI
While not typically part of CodeReview, the UI should give you all the hints needed to play the game. The fact that 'R' should be pressed should be mentioned, even more important, this
if (keyboard_event.keyCode == 82) {could have been this:
//Rotate the arrow when the player presses 'R'
if (keyboard_event.keyCode == 82) {or even this:
var ROTATE_KEY = 82; //Upper case R rotates the arrow
..
if (keyboard_event.keyCode == ROTATE_KEY ) {Also, I don't know why the user has to click the colored box, is that to prevent accidental clicks? It was not intuitive to me.
Constants
- Declaring
UP,RIGHT, andLEFTis good, but you still use the constants45and315in your code. Also, it is not clear to me why you would go for315but not for135. A minor comment indicating that these are angles would have been great as well
var game = new Game_Controller(9, 9, 1);
Style
- bad :
this.win_position['column'], good :this.win_position.column
- really bad :
number_of_rows, better:numberOfRows, even betterrowCountbecause it is less wordy and because it is lowerCamelCase
- Use lowerCamelCase at all times!
*
JsHint.com
- Always use JsHint.com or hit the JsHint button in http://jsfiddle.net/
- Use old skool loops for iterating over arrays, not
for(key in array){like you do, it will prevent head aches later.for(key in array){should be reserved for iterating over object properties.
- Declare all your variables with
var, some likecell_columnare now polluting the global namespace
- It seems you have a number of unused variables, you should clean that up
Objects
It is more idiomatic in JavaScript to forego getters and setters until you actually need them, so this:
function Cell(row, column) {
this.row = row;
this.column = column;
this.direction = null;
this.get_row = function () {
return this.row;
};
this.get_column = function () {
return this.column;
};
this.get_direction = function () {
return this.direction;
};
this.set_direction = function (new_direction) {
this.direction = new_direction;
};
}Should really only be this:
function Cell(row, column) {
this.row = row;
this.column = column;
}If you ever need processing within getters and setters, deal with it then. I am willing to bet that you will never need it for this project. I left out
direction, keeping the value of direction as undefined which is also more idiomatic than assigning null and more memory friendly.Furthermore, when you wrap an object around an array like you do for
Board, then last thing you want to provide is something like this:this.get_board = function () {
return this.board;
};It means some code will be doing things that the Board class should be doing. Fortunately this function is never called and I could just remove it. Read up on YAGNI.
Furthermore, I have some issues with functions like these:
this.get_cell_direction = function (row, column) {
var cell = this.board[row][column];
return cell.get_direction();
};The board has no business knowing the direction of a cell, it knows its own next arrow direction and that should be it. It could be argued that this was done for syntactic sugar but I dont see much of a benefit when I compare these:
board.get_cell_direction( row , column )vs
board.cell(row, column).directionOn directions
You should read up on DRY, it is a great engineering (and life) principle that you could apply to this code:
this.rotate_next_arrow = function(){
var new_direction;
if(this.next_direction == UP){
this.next_direction = LEFT;
} else if (this.next_direction == LEFT){
this.next_direction = RIGHT;
} else {
this.next_direction = UP;
}
return this.next_direction;
};
this.calculate_next_cell_location = function (row, column, direction) {
row--;
if (direction == 315 && column > 0) {
column--;
} else if (direction == 45 && column < number_of_column - 1) {
column++;
}
if (row < 0) {
return null;
} else {
return { "row": row, "column": column };
}
};You could simply encode the sequence of directions so that
rotate_next_arrow becomes a single statement, you could also encode the column delta's to enhance calculate_next_cell_location:var directions = {};
directions[UP] = { columnDelta = 0, next : LEFT };
directions[LEFT] = { columnDelta = -1, next : RIGHT};
directions[RIGHT] = { columnDelta = +1, next : UP };
this.rotateArrow = function(){
this.direction = directions[this.direction].next;
return this.next_direction;
};I also renamed
rotate_next_arrowCode Snippets
if (keyboard_event.keyCode == 82) {//Rotate the arrow when the player presses 'R'
if (keyboard_event.keyCode == 82) {var ROTATE_KEY = 82; //Upper case R rotates the arrow
..
if (keyboard_event.keyCode == ROTATE_KEY ) {function Cell(row, column) {
this.row = row;
this.column = column;
this.direction = null;
this.get_row = function () {
return this.row;
};
this.get_column = function () {
return this.column;
};
this.get_direction = function () {
return this.direction;
};
this.set_direction = function (new_direction) {
this.direction = new_direction;
};
}function Cell(row, column) {
this.row = row;
this.column = column;
}Context
StackExchange Code Review Q#51266, answer score: 2
Revisions (0)
No revisions yet.