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

Conversation

bluemittens505
Copy link

This code is mostly working, but it has a bug. When you click on the same tile twice, the code considers the "tiles matching" and removes the tile. I tried checking if this.$td and tile.$td were equal in Tile.prototype.matches(), but I didn't get the results I expected. Is there a way to prevent this bug? Or is my whole design flawed? :-(

My first revision - still some kinks to work out
Has a bug.  When you click on the same tile twice, the code considers
the "tiles matching" and the tile disappears.
Fixed some errors displayed when I created the pull request.
@afeld
Copy link
Member

afeld commented Feb 16, 2016

is my whole design flawed?

Not at all – this looks really good!

I tried checking if this.$td and tile.$td were equal in Tile.prototype.matches(), but I didn't get the results I expected

Ah. Remember in the first class when we were talking about different types in JavaScript, and how two objects that look the same aren't triple-equal (===)? For example:

var a = {};
var b = {};
a === b; // false

You ran into that exact issue here, because clicked1.$td is not the same (jQuery) object as clicked2.$td, even though they contain the same element object under the hood (in a property). This happened because $(this) separately for each Tile that is created. Does that make sense? There are several ways to fix this, each with a simplified example:

  1. Only create the jQuery object once.

    // change
    game.clicked1 = new Tile($(this));
    game.clicked2 = new Tile($(this));
    // to
    var $tile = $(this);
    game.clicked1 = new Tile($tile);
    game.clicked2 = new Tile($tile);
  2. Compare the underlying element object rather than the jQuery object. Like an array, you can use square brackets to access the underlying elements from a jQuery object.

    // change
    clicked1.$td === clicked2.$td
    // to
    clicked1.$td[0] === clicked2.$td[0]
  3. Use jQuery's .is() method to compare two jQuery objects directly.

    // change
    clicked1.$td === clicked2.$td
    // to
    clicked1.$td.is(clicked2.$td)

Hope that helps? Remind me to bring this up in class – it's tricky!

Fixed bug with the help of Aidan.  Thanks.  Fixed cursor type on div
tags.  Re-factored some of the larger functions.  Still need to fix the
action().
Add missing semicolons
Add New Game button.  Refactored some more functions.
Moved most of code out of document.ready()
Re-arrange order of functions
This reverts commit 38eec98.
Final version on Tuesday night.  Hopefully more can be done tomorrow
night.
Added another class for buttons
Added another board to make sure all objects operate independently
More tweaks
More tweaks to pass Travis CI tests
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.

@afeld
Copy link
Member

afeld commented Mar 3, 2016

Left a bunch of nitpicky comments, but great work overall!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants