Skip to content
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

Memory Homework #47

Open
wants to merge 12 commits into
base: gh-pages
Choose a base branch
from
Open
27 changes: 22 additions & 5 deletions index.html
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
<!DOCTYPE html>
<html>
<head>
<title>Memory exercise</title>
</head>
<body>
</body>
<head>
<title>Memory exercise</title>
<link href="memory.css" rel="stylesheet" />
<script src="http://code.jquery.com/jquery-2.1.0.min.js"></script>
<script src="memory.js"></script>
</head>
<body>
<div id="game">
<h1>Memory Game #1</h1>
<div id="msg"></div>
<div id="board"></div>
<button id="new">New Game</button>
<button id="reset">Reset</button>
</div>
<div id="game2">
<h1>Memory Game #2</h1>
<div id="msg2"></div>
<div id="board2"></div>
<button id="new2">New Game</button>
<button id="reset2">Reset</button>
</div>
</body>
</html>
34 changes: 34 additions & 0 deletions memory.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#game {
float: left;
width: 50%;
}
#game2 {
float: right;
width: 50%
}
h1 {
color: #000066;
}
p {
color: #0000CC;
font-size: 20px;
}
.tile, .tile2 {
height: 100px;
width: 100px;
border-radius: 100%;
background-color: #F5680A;
}
.tile div, .tile2 div {
font-size: 40px;
text-align: center;
color: #FFFFFF;
opacity: 0;
cursor: default;
}
button {
margin: 20px;
padding: 10px;
color: #000066;
font-size: 20px;
}
201 changes: 201 additions & 0 deletions memory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
var Game = function(boardElements) {
this.boardElements = boardElements;
this.tileClass = boardElements.tileHtmlElementClass.substr(1);
this.maxValue = boardElements.height*boardElements.width/2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't be afraid of whitespace within lines 😉 Makes things more readable.

this.takenValues = "#";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use an array?

this.values = null;
this.clicked1 = null;
this.clicked2 = null;
this.remainingMatches = boardElements.height*boardElements.width/2;
};

Game.prototype.taken = function(value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: methods that return true/false are often prefixed with is, i.e. isTaken().

var taken = true;
var index = this.takenValues.indexOf("#" + value + "#");
var lastIndex = this.takenValues.lastIndexOf("#" + value + "#");
if ( index === -1 || index === lastIndex ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably warrants an inline comment, to explain what this is checking for.

taken = false;
}
return taken;
};

Game.prototype.genValue = function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name could be more descriptive...maybe generateUniqueValue?

var found = false;
var value = -1;
while ( !found ) {
value = Math.floor(Math.random()*this.maxValue);
if ( !this.taken(value) ) {
found = true;
this.takenValues += value + "#";
}
}
return value;
};

Game.prototype.genValues = function() {
var values = [];
for (var i = 0; i < this.boardElements.height; i++) {
values[i] = [];
for (var j = 0; j < this.boardElements.width; j++) {
values[i][j] = this.genValue();
}
}
return values;
};

Game.prototype.genHtml = function() {
var board = "<table>";
for (var i = 0; i < this.boardElements.height; i++) {
board += "<tr>";
for (var j = 0; j < this.boardElements.width; j++) {
board += "<td class=\"" + this.tileClass + "\"><div>" + this.values[i][j] + "</div></td>";
}
board += "</tr>";
}
board += "</table>";
return board;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know, there might be enough logic in this to warrant its own class, e.g. a BoardGenerator or TileValuesGenerator or something. Then the Game would put all of it together and manage the actual tiles.


Game.prototype.unsetClickedTiles = function() {
this.clicked1.hideTile();
this.clicked1 = null;
this.clicked2.hideTile();
this.clicked2 = null;
};

Game.prototype.removeClickedTiles = function() {
this.clicked1.removeTile();
this.clicked1 = null;
this.clicked2.removeTile();
this.clicked2 = null;
};

Game.prototype.checkWinner = function() {
var self = this;
if ( --self.remainingMatches === 0 ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this line works, it would be easier to read if it were split to two lines.

setTimeout(function () {
$(self.boardElements.msgHtmlElementId).append("<p>***WINNER***</p>");
},775);
}
};

var Tile = function($td) {
this.$td = $td;
this.$div = $td.find('div');
this.value = $td.find('div').text();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: you could use this.$div.text() so that it doesn't need to search the DOM again.

};

Tile.prototype.matches = function(tile) {
var match = false;
if ( this.value === tile.value ) {
match = true;
}
return match;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would simplify this to

return this.value === tile.value;

};

Tile.prototype.showTile = function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: it's implied that the method is acting on a Tile, so I would probably just name this show.

this.$div.css('opacity','1');
};

Tile.prototype.hideTile = function() {
this.$div.animate({opacity: 0},750);
};

Tile.prototype.removeTile = function() {
this.$td.fadeOut(750);
};

Tile.prototype.sameTileAs = function($tile) {
return this.$td.is($tile);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is essentially doing the same thing as Tile.prototype.matches, I would see if you can change your code to only need one or the other.


var setClicked = function($tile) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could you avoid making this function global?

var clicked = new Tile($tile);
clicked.showTile();
return clicked;
};

var action = function(event) {
var $tile = $(this);
var game = event.data.game;
if ( game.clicked1 === null ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than checking for null specifically, it's more common to write if ( !game.clicked1 ).

game.clicked1 = setClicked($tile);
} else if ( !game.clicked1.sameTileAs($tile) ) {
game.clicked2 = setClicked($tile);
if ( game.clicked2.matches(game.clicked1) ) {
game.removeClickedTiles();
game.checkWinner();
} else {
game.unsetClickedTiles();
}
}
};

Game.prototype.initBoard = function(chgvalues) {
var self = this;
if ( chgvalues ) {
self.takenValues = "#";
self.values = self.genValues();
}
$(self.boardElements.boardHtmlElementId).empty();
$(self.boardElements.boardHtmlElementId).append(self.genHtml());
$(self.boardElements.tileHtmlElementClass).click({
game:self
},action);
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep this grouped with the other Game.prototype methods above.


var Button = function(type,htmlElementId) {
this.type = type;
this.htmlElementId = htmlElementId;
};

Button.prototype.initButton = function(game) {
var chgvalues = false;
if ( this.type === "New" ) {
chgvalues = true;
}
$(this.htmlElementId).click(function() {
$(game.boardElements.msgHtmlElementId).empty();
game.initBoard(chgvalues);
game.clicked1 = null;
game.clicked2 = null;
game.remainingMatches = game.boardElements.height*game.boardElements.width/2;
});
};

var gameSetup = function(gameElements) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: I would change this to a verb, e.g. setUpGame.

var game = new Game({
height: gameElements.height,
width: gameElements.width,
boardHtmlElementId: gameElements.boardHtmlElementId,
msgHtmlElementId: gameElements.msgHtmlElementId,
tileHtmlElementClass: gameElements.tileHtmlElementClass
});
game.initBoard(true);
var newButton = new Button("New",gameElements.newButtonHtmlElementId);
newButton.initButton(game);
var resetButton = new Button("Reset",gameElements.resetButtonHtmlElementId);
resetButton.initButton(game);
};

$(document).ready(function() {
// Code only works for boards with an even number of tiles
gameSetup({
height: 3,
width: 6,
boardHtmlElementId: "#board",
msgHtmlElementId: "#msg",
tileHtmlElementClass: ".tile",
newButtonHtmlElementId: "#new" ,
resetButtonHtmlElementId: "#reset"
});
gameSetup({
height: 4,
width: 4,
boardHtmlElementId: "#board2",
msgHtmlElementId: "#msg2",
tileHtmlElementClass: ".tile2",
newButtonHtmlElementId: "#new2" ,
resetButtonHtmlElementId: "#reset2"
});
});