3
\$\begingroup\$

Mostly for the experience (but also so I could waste hours playing it), I made a simple memory card game in JavaScript. I built it so it has a backbone framework that could be easily extended by stripping off my actual implementation. It's the backbone I'd like reviewed here, and mostly:

  1. My OO - anything wrong; anything that could be improved on?
  2. Obvious code efficiency: is there anything that could make it more efficient?
  3. Structure: Is the structure of the game a good idea?

var Card = (function() {
    var self = Object.create({}, {
        val: {
            value: -1
        },
        index: {
            value: -1
        },
        addTo: {
            value: function(game) {
                var random = -1;
                counter = 0; //break counter to stop infinite loop. :) 
                while ((game.cards[random] !== undefined && counter <= 100) | random === -1) {
                    random = Math.round(game.cards.length * Math.random());
                    counter++;
                }
                this.index = random;
                game.cards[random] = this;
            }
        },
        isMatch: {
            value: function(game) {
                if (this.val == game.selected.val) {
                    game.matches++;
                    return true;
                }
                return false;
            }
        }
    });
    return self;
})();
var Game = (function() {
    var self = Object.create({}, {
        cards: {
            value: new Array(30)
        },
        matches: {
            value: 0
        },
        init: {
            value: function(func) {
                for (i = 0; i < this.cards.length / 2; i++) {
                    var card = Object.create(Card, {
                        val: {
                            value: i
                        }
                    });
                    var card2 = Object.create(Card, {
                        val: {
                            value: i
                        }
                    });
                    card.addTo(this);
                    card2.addTo(this);
                }
                if (typeof func === 'function') {
                    func();
                }
            }
        },
        selected: {
            value: null
        }
    });
    return self;
})();

Sorry about the length; that's partly why I want it reviewed.

To see how to implement it and the full code, see here

EDIT: FYI, I realize the game isn't currently extensible. For example, if I wanted there to be multiple types of matches, I can't do that quite yet. I'm pretty sure I know how to add that in, but I wanted to make sure my design structure was solid first.

\$\endgroup\$
1
\$\begingroup\$

I'll start off with some questions, because you are using techniques, that I don't have much experience with.

  • What run time environment are you considering using? (Or in other words, which browsers do you want to support?) Object.create is very, very new and thus support isn't very wide spread.

  • Why do you wrap all properties and methods in their own "sub-objects"?

Both points together lead to (IMHO) very unwieldy code such as

var card = Object.create(Card, {
    val: {
        value: i
    }
});

which would be much simpler with "normal" objects:

var card = new Card(i);

Speaking of which, using such a "big" class to represent a simple card sees a bit excessive to me, but if it's just for practice it's ok.

However even for practice the addTo method is badly written. It's probably better to put all cards into the array in order of creation and then shuffle the array. Have a look at Fisher-Yates which is considered the "standard" algorithm for shuffling.

\$\endgroup\$
2
\$\begingroup\$

First I see that both classes depend on each other and are tightly coupled together. Why does a Card object need to know its own index in an Array?

Secondly, the Card object updates the game object, but the game object contains an array of the cards, which means the Game object owns a bunch of Card objects.

If you were to remove the dual dependency, then you could rewrite your isMatch method to be

    isMatch: {
        value: function(inCard) {
            return (this.val == inCard.val);
        }
    }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.