Reputation: 33
This is my current code for a memorygame I'm working on. I'm using pure JavaScript and I'm having trouble with the onclick function of my a-tags. What I want is for the game to swap the clicked image (question mark) with the image assigned to the clicked images' class number, thus creating a flip effect.
What I currently get is that, no matter where I click only the last image gets swapped.
"use strict"
var newDiv = null;
var innerDiv = null;
var table = null;
var row = null;
var cell = null;
var aTag = null;
var image = null;
// Use this to put class names on the images.
var boxCounter = 0;
// Static memory-object.
var Memory = {
memoryArray: [],
init: function (e) {
// Calls a separate js-file which generates a random numbers.
Memory.memoryArray = RandomGenerator.getPictureArray(4, 4);
// Calls the rendering method
Memory.renderArray(Memory.memoryArray);
},
renderArray: function (myArray) {
// Creates and places div-tags in the HTML-document.
newDiv = document.createElement("div");
document.getElementsByTagName("body")[0].appendChild(newDiv);
innerDiv = document.createElement("div");
newDiv.appendChild(innerDiv);
// Creates a table and places it in the HTML-document.
table = document.createElement("table");
table.border = 1;
// Generates rows and cells, swap the 4's to change the size of the gameboard.
for (var i = 0; i < 4; ++i) {
row = document.createElement("tr");
table.appendChild(row);
// Creates a cell, each with its own respective random number.
for (var j = 0; j < 4; ++j) {
cell = document.createElement("td");
// Adds a "Question-mark"-picture to the cell and places them in a-tags.
image = document.createElement("img");
image.className = myArray[i * 4 + j];
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/0.png?raw=true";
aTag = document.createElement("a");
aTag.onclick = function () {
Memory.flipTile(image.className);
};
// Places the pictures in the document, along with its random number for easier testing purposes.
aTag.appendChild(document.createTextNode(myArray[i * 4 + j]));
aTag.appendChild(image);
cell.appendChild(aTag);
row.appendChild(cell);
};
};
innerDiv.appendChild(table);
},
flipTile: function (imageClass) {
console.log(imageClass);
// This should flip the tiles if the number matches the class name.
if (imageClass == 1) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/1.png?raw=true";
};
if (imageClass == 2) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/2.png?raw=true";
};
if (imageClass == 3) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/3.png?raw=true";
};
if (imageClass == 4) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/4.png?raw=true";
};
if (imageClass == 5) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/5.png?raw=true";
};
if (imageClass == 6) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/6.png?raw=true";
};
if (imageClass == 7) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/7.png?raw=true";
};
if (imageClass == 8) {
image.src = "https://github.com/1dv403/1dv403-laborationer/blob/master/3-gameon/memory/pics/8.png?raw=true";
};
}
};
window.onload = Memory.init;
Upvotes: 0
Views: 1684
Reputation: 27823
All your click handlers reference the same image variable (which after the for loop ends will be the last image). You need each handler to reference a different image variable. You can do that by creating a copy of it by passing it as parameter to a function.
Replace this:
function () {
Memory.flipTile(image.className);
};
With this:
function(classname) {
return function () {
Memory.flipTile(className);
};
}(image.classname);
Second problem: global image. The flipTile function does something to an image variable, but none is declared inside it. You actually want to pass the image as a parameter, like so:
Bad:
flipTile: function (imageClass) {
Good:
flipTile: function (image, imageClass) {
Now you also need to change the calls to the function to pass the image:
function(img, classname) {
return function () {
Memory.flipTile(img, className);
};
}(image, image.classname);
Considering that you passed the image as parameter to flipTile, it doesn't seem like you need to pass image.classname, so you could change the function to use image.classname instead. Thais step is optional, the code should work fine without it.
Upvotes: 0
Reputation: 66334
This is a classic problem involving closure and loops. Consider the closure created by your function expression
aTag.onclick = function () {
Memory.flipTile(image.className);
};
It's referencing image
from a higher scope, so what happens the next iteration in the loop?
image = document.createElement("img");
image
now points to a different Object, so by the last iteration of your loop, all image
s point to the same (last) <img>
element.
You will need to create a closure for image
so this doesn't happen. You can do this by writing a function to generate your desired function
function makeClickListener(elm) {
return function () {
Memory.flipTile(elm.className);
};
}
Now, use this to create your click listeners
aTag.onclick = makeClickListener(image);
This could be further optimised by passing only the String rather than the whole Object (less memory required).
Upvotes: 1