2

Why is my function running multiple times? 

var s,
getData = {

    settings: {
        gender: $("input[name='gender']"),
        age: $("input[name='age']")
    },

    init: function() {
        s = this.settings;
        this.getInput();
    },

    getInput: function() {
        for (i in s) {
            s[i].on("click", function() {
                s[i].off();
                console.log(this.getAttribute('value'))
            });
        }
    },
};

What I find odd is that when I don't click the same input twice it only runs once. For instance if I click 'male' and then 'female' it would show in the console 

(1) male
(2) female

which logically makes sense to me, if I clicked 'male' 5 times then I get

(14) male

and it just gets larger and larger.

EDIT: JSFiddle If you open console you can see what happens

Improve this question
6
  • 1
    why the complicated code ? Commented Sep 23, 2016 at 4:08
  • post you code ? Commented Sep 23, 2016 at 4:11
  • @madalinivascu why is it complicated? It's modular code, which is cleaner imo. Commented Sep 23, 2016 at 4:29
  • @passion I posted the code in a JSFiddle Commented Sep 23, 2016 at 4:30
  • @madalinivascu is right this is way too complicated. See my answer for the solution Commented Sep 23, 2016 at 4:38

4 Answers 4

Reset to default
3

What's happening is that getInput is being called for each click event, meaning that the loop is running each time, registering the event again. So each time you click the radio button the loop will attach an event to the DOM element again. You should only register the event once: https://jsfiddle.net/2jbLdo9n/

Remove the onclick event on the input:

<div class="right-item">
  <input id="male" type="radio" name="gender" value="male">
  <label class="left-m" for="male"><span></span> Male </label>
</div>
<div class="right-item">
  <input id="female" type="radio" name="gender" value="female">
  <label for="female"><span></span> Female </label>
</div>

Then in your JavaScript, just call getData.init();

getData.init();
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

@DanielContreras see my answer.
oh I hadn't realized that onclick worked that way. I'll try it without onclick() in html. But how would the form dynamically retrieve any changes? My script isn't inline like the JSFiddle, I just did it that way because the getInput function wasn't working. However, the javascript wouldn't continuously listen would it?
@DanielContreras Each time the form is clicked, the values will be retrieved dynamically. And you'll have access to the value in the click handler, the one that is in your loop.
@DanielContreras .on("click",function(){}) is the attribute onclick that you are asked to remove so the on event continuously listens to find a click event
@madalinivascu I think he is confusing angular style dynamic things with regular plain javascript events :)
|
2

bind a single click event, if you are calling this multiple time then first remove the click event and the append it once again or use one()

try the following:

var s,
getData = {

    settings: {
        gender: "input[name='gender']",
        age: "input[name='age']"
    },

    init: function() {
        s = this.settings;
        this.getInput();
    },

    getInput: function() {

            $(s.gender+','+s.age).on("click", function() {
                console.log($(this).val());
            });
    },
};

remove the click event and initiate the call at document ready

see demo: https://jsfiddle.net/qzrfwc3g/

or do it the normal way 3 lines of code: 

$("input[name='gender'],input[name='age']").on("click", function() {
     console.log($(this).val());
});
Improve this answer

6 Comments

I tried the .one() function but it still has odd behavior. However, it did stop the multiple calls to my function.
what do you mean by odd?
wait you are creating a click event from a click event, why?
He should only register the event once, not multiple times like he's doing.
I suppose the reason I tried it with a loop is because I have multiple inputs and when I did it that way all I saw was repeated code. I guess what I really wanted to do was have a way that my document can listen to all inputs without writing out 100 lines of code. Maybe I'm wrong and can't do that though?
|
1

I don't know why you want do it like this , as you code , I think there is a error here .

for (i in s) {
  s[i].on("click", function() {
      s[i].off();
      console.log(this.getAttribute('value'))
   });
}

You declare i as global , when event fires , i refers the same value .You can try this : 

for (let i in s) {
      s[i].on("click", function() {
          s[i].off();
          console.log(this.getAttribute('value'))
       });
    }
Improve this answer

1 Comment

You're right, the i variable was not being reset after every function call. I wasn't aware that my variable was a global variable. I'm fairly new to JS so I'm still trying to learn it. Thank you for your advice.
0

you are adding multiple click events to same element that why you get multiple console.log for one click. To resolve this just add a condition before assigning event listners.

var s,
    getData = {

      cacheDOM: {
        gender: $("input[name='gender']"),
        age: $("input[name='age']")
      },

      init: function() {
        if(!s){
            s = this.cacheDOM;
            this.getInput();
        }
      },

      getInput: function() {
        for (i in s) {
          s[i].on("click", function() {
            s[i].off();
            console.log(this.getAttribute('value'))
          });
        }
      },
    };
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.