Why all my JavaScript buttons return letter Z?

Question

4.00/5 (1 vote)

See more:

I am making Hangman in JavaScript for self-learning

I added alphabet buttons and they appear correctly but all buttons when clicked return Z, why?

C#

function set_controls(){
    text_tries=document.getElementById('text_tries');
    var divalpha = document.getElementById('alpha_buttons');
    var letters = alphabet.split("");
    for(var i=0;i<alphabet.length;i++){
        if(i%9==0){
            var br = document.createElement('br');
            divalpha.appendChild(br);
        }
        var b = document.createElement('button');
        var ch = letters[i];
        var str = ch.toString();
        var b_id=str+'btn';
        debug('add button id=' + b_id);
        b.setAttribute('id',b_id);
        b.innerHTML=str;
        b.style.backgroundColor="yellow";
        b.style.color="black";
        b.style.fontWeight="bold";
        divalpha.appendChild(b);
        b.onclick=function(){
            submit_letter(b_id)
        };

    }
}

function init_hangman(){
    set_controls();
    hangman_contest.fx.new_game();
}

function submit_letter(btn_id){
    debug('clicked button id=' + btn_id);
    var letter=document.getElementById(btn_id).innerHTML;
    debug(letter);
    hangman_contest.fx.add_letter(letter);
    var tries=document.getElementById('text_tries');
    tries.value=hangman_contest.usedletters.join("");
}

It seems the click event created for button Z is pointing to all buttons???

Posted 28-Aug-15 16:32pm

BacchusBeale

Updated 28-Aug-15 16:38pm

v3

Add a Solution

Comments

Sergey Alexandrovich Kryukov 29-Aug-15 1:39am

Not event, but event handler. Wrong idea. Of course, but it should be assigned to all instances; this is the whole idea, to reuse the handler function object. The problem is different: b_id value. I gave you comprehensive explanation and solution, please see Solution 1.
—SA

2 solutions

Solution 2

Wow! Even though the main question is fully answered in Solution 1, after some thinking, I invented a very different approach to such problems, more reliable and flexible, because you don't have to create extra coupling between buttons attributes, HTML or any other properties, so you can extract the letter directly from the handler.

At the same time, it's more subtle and the understanding requires deeper understanding of JavaScript. You should understand that functions objects can be used as other objects, so you can add any properties for them. So, this is what you can do inside your loop:

C#

var b_id = //...
var functionObject = function() {
    submit_letter(arguments.callee.letter);
}
functionObject.letter = "A";
b.onclick = functionObject;

As you can see, your letter submitted is totally decoupled from the button objects. There is only one point or binding between the button objects and function objects, the property onclick.

This solution has one big problem though: using callee is not allowed in strict mode:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode[^],
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments/callee[^].

I want to say thank you: your rather naive question inspires interesting thoughts and is related to deeply fundamental things.

—SA

Posted 28-Aug-15 21:03pm

Sergey Alexandrovich Kryukov

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Sergey Alexandrovich Kryukov · Accepted Answer · 2015-08-28T19:23:00

The bug is way too trivial: you are using b_id in submit_letter(b_id). This is one object shared by all the buttons. What is its value? The values assigned to this variable at the last iteration, 'Z'. One interesting question is this one: how this value, accessible via a local variable (that is, a variable on stack, normally) can get to the event handler object. This question is not trivial at all: this is the effect of closure. It is very important to learn this important advanced aspect of programming: http://en.wikipedia.org/wiki/Closure_(computer_programming)[^].

Understanding of closures in not so easy. If something is not clear in this article, please ask a question; I'll explain.

Here is how you can understand what's going on. Consider order of execution. In your loop, you only create function objects and assign them to the property onclick. You never call those functions before you complete the loop. Those functions are called a lot later, when the event is invoked as a result of the user's click. And when the call is performed b_id is used. Normally, this object would not even exist, as a local variables after leaving the stack frame. But the effect of closure (due to the use of the variable inside the handler) preserves this variable, so it could be used in the handler. And what is it's value? The value assigned back them, at the last iteration of the loop execution. Z.

Some more problems:
You don't need to split alphabet. To bad you did not show its definition, but I'm almost sure it is something like the string "ABCD....F". You can access each character directly, as alphabet[index]. Also, it's quite a bad idea to concatenate "btn". The attribute id value could better be "A", "B", etc. Moreover, you could assign some other attribute and use it, for example name, it would relax the uniqueness requirements. But your main problem is: you never use the attribute values assigned.

So, what would be the correct solution? Pretty simple: you need to use the argument of the button click handler. Let's forget my advice to assign any attribute values to the letters of alphabet. You could simply assign button texts (innerHTML) to those letters, or letters with some markup. Let's assume there are just letters, for simplicity (otherwise you can always extract that letter from innerHTML string). Then your handler may look like this:

JavaScript

b.onclick = function(eventInstance) {
   submit_letter(eventInstance.target.innerHTML)
}

Note that eventInstance.target is the reference to your button which invoked the event. If you want to use some attributes, you can extract the attribute value from this object.

Please see:
https://developer.mozilla.org/en-US/docs/Web/API/Event[^],
https://developer.mozilla.org/en-US/docs/Web/API/Event/target[^].

—SA