Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

#Game design

Game design

###Don't rush me

Don't rush me

###Hey, I wanna see my score!

Hey, I wanna see my score!

#Design

Design

###Slicing the methods right down the middle

Slicing the methods right down the middle

###An ID for you, an ID for you, oh; an ID for you, too.

An ID for you, an ID for you, oh; an ID for you, too.

#Nitpickery

Nitpickery

###Do the math

Do the math

###There's a bump in your indentation

There's a bump in your indentation

###Defining the undefined

Defining the undefined

###Weakqual

Weakqual

###What is wrong in this picture?

What is wrong in this picture?

#Game design

###Don't rush me

###Hey, I wanna see my score!

#Design

###Slicing the methods right down the middle

###An ID for you, an ID for you, oh; an ID for you, too.

#Nitpickery

###Do the math

###There's a bump in your indentation

###Defining the undefined

###Weakqual

###What is wrong in this picture?

Game design

Don't rush me

Hey, I wanna see my score!

Design

Slicing the methods right down the middle

An ID for you, an ID for you, oh; an ID for you, too.

Nitpickery

Do the math

There's a bump in your indentation

Defining the undefined

Weakqual

What is wrong in this picture?

Source Link
SirPython
  • 13.4k
  • 3
  • 38
  • 93

#Game design

###Don't rush me

Why do the letters disappear after a few a seconds? What if I need time to think?

To me, this seems like a "hacky way" to hide the cards after two were chosen. Also, this even leads to a problem that occurs after a pair is found: the space I click on next shortly disappears, even though I just clicked it.

After a pair is found, you should clear the timeout with clearTimeout so the next spaces that are clicked do not disappear.


###Hey, I wanna see my score!

I see this in your code:

if (pair == 8) {
 alert("Tries: " + tries);
}

But, when I match all the squares, I don't get an alert showing my tries (and it's not just because I have popups blocked).


#Design

###Slicing the methods right down the middle

$box1.removeClass("show").addClass("hidden");

You seem to be repeating this pattern of removeClass and then addClass a lot. In jQuery, this is a method called toggleClass that will add the class if it does not exist in the element, and will remove it if it does.

To simplify your code and cut the method calls in half, you should set the styles of hidden or shown to be the default styles of the buttons. Then, when you want to apply the other class, use toggleClass. The applied class will override the default styles.

This should speed up your code.


###An ID for you, an ID for you, oh; an ID for you, too.

An ID for each button? That's a really bad design.

A better design would be to identify them by something that they all share and is unique to these buttons. In this case, since these are the only buttons on your page, you can identify them with their tag name:

$("button");

#Nitpickery

###Do the math

function load(cardSet) {
 var index = 0;
 for (i = 0; i < 4; i++) {
 for (j = 0; j < 4; j++) {
 $("#b" + i + j).text(cardSet[index]);
 index++;
 }
 }
}

Having that extra index variable is unnecessary. You can just use the variables you already have: i and j and the number 4.

$("#b" + i + j).text(cardSet[i * 4 + j]);

###There's a bump in your indentation

You are being inconsistent with your indentation. In some places, you are indenting two spaces. In others, four. I recommend that you stick with one (I like four).

###Defining the undefined

var $box1 = undefined;

In JavaScript, it's sorta bad practice to set a variable to undefined, because that is also the value you get for something that has not yet been defined or has no definition.

This can make your code confusing. Instead, I recommend that you use null. This shows that the value had been defined at some point in time, just there is not supposed to be any value in it.


###Weakqual

In JavaScript, these are bad.

==

or

!=

While they are still comparing two values, these both will cast the second value to be the same type as the first value, which can result in conditions that really shouldn't pass.

You should always use

===

or

!==

###What is wrong in this picture?

numberOfClicks += 1;

Whoa! Where did that come from? All of a sudden increasing a value that was not previously declared anywhere? That's not good.

I have no idea what you are trying to do here, but it looks like you meant to use the variable tries.

lang-css

AltStyle によって変換されたページ (->オリジナル) /