0

I have a function which needs to be called when the Event is triggered, but for some reason I am getting syntax errors. It used to work fine when I put the function inside the event listener, but now I need to be able to call it instead.

rbProject.addEventListener('change', rbProjectClicked()) { // LINE WITH ERROR

})

function rbProjectClicked(){

    txtReference.placeholder = "Insert project reference";
    txtTitle.placeholder = "Insert project title";
    txtReference.value = ""
    txtTitle.value = "";
    txtReference.disabled = false;
    txtTitle.disabled = false;
    document.getElementById('divProjectManager').style.display = '';
    document.getElementById('divAccountManager').style.display = 'none';

    document.getElementById('divAccountFeedback').style.display = 'none';
    document.getElementById('divClientFeedbackOver').style.display = '';
    document.getElementById('divClientFeedbackUnder').style.display = '';
}
MRC
  • 555
  • 2
  • 10
  • 30

2 Answers2

1

You are binding the result of rbProjectClicked() to the change event, rather than a function reference.

Do this instead:

rbProject.addEventListener('change', rbProjectClicked);

Notice, I removed the trailing () from rbProjectClicked. The () invokes the function call, but you only want to bind a reference to the function. rbProjectClicked is a reference to the function.

crush
  • 16,713
  • 9
  • 59
  • 100
  • 1
    Also this: http://mislav.uniqpath.com/2010/05/semicolons/ semi-colon are kind of optional. – Loïc Faure-Lacroix Feb 04 '14 at 14:59
  • Fair enough. Part of the confusion that article addresses is from developers, like myself, who use multiple languages every day, many of which require semi-colons. – crush Feb 04 '14 at 15:03
  • @Joey Show me what you tried because it absolutely works if you did it right. – crush Feb 04 '14 at 15:33
1

The problem is here:

rbProject.addEventListener('change', rbProjectClicked()) { // LINE WITH ERROR

})

In javascript, there are two ways to declare a function, you can declare an anonymous function like this:

function (params ...) {...}

Or a named function:

function name(params ...) { ... }

Named declaration are "interpreted first" so the order in which you declare them doesn't matter. If you did wrote

rbProject.addEventListener('change', rbProjectClicked);
var rbProjectClicked = function () { ... }

It wouldn't work. The following snippet would work. It is usually a good idea to declare things before you actually use them, but it's a matter of taste I guess:

rbProject.addEventListener('change', rbProjectClicked);
function rbProjectClicked() { ... }

What you really want is this:

function rbProjectClicked(){
    txtReference.placeholder = "Insert project reference";
    txtTitle.placeholder = "Insert project title";
    txtReference.value = ""
    txtTitle.value = "";
    txtReference.disabled = false;
    txtTitle.disabled = false;
    document.getElementById('divProjectManager').style.display = '';
    document.getElementById('divAccountManager').style.display = 'none';

    document.getElementById('divAccountFeedback').style.display = 'none';
    document.getElementById('divClientFeedbackOver').style.display = '';
    document.getElementById('divClientFeedbackUnder').style.display = '';
}


rbProject.addEventListener('change', rbProjectClicked);

But, we're not finished yet. As I can see, you appear to have mixed up an anonymous function with a named function.

What you wanted to type is this probably:

rbProject.addEventListener('change', function () {
   // code here
});

As you can see, I declared a function as a parameter to addEventListener. A reason to use anonymous function is purely syntactic. It is sometimes easier to read.

Loïc Faure-Lacroix
  • 13,220
  • 6
  • 67
  • 99
  • 1
    ***You don't have to declare the function first. [Take a look](http://jsfiddle.net/Up7v9/).*** – crush Feb 04 '14 at 14:50
  • 1
    You don't have to bind an anonymous function to an event listener either. A declared function works just fine. It's never faster. You are simply giving `addEventListener` a reference to a function. – crush Feb 04 '14 at 14:51
  • I didn't say he had to, I say he can do that. and notice I wrote that it's purely syntactic. – Loïc Faure-Lacroix Feb 04 '14 at 14:51
  • *"As I can see, you appear to have mixed up an anonymous function with a named function. What you wanted to type is this probably.* By what evidence do you draw this conclusion? – crush Feb 04 '14 at 14:52
  • 1
    You wrote that it's easier to read and slightly faster. In fact, it's not any easier to read, it's never slightly faster, and can actually be less optimal if you use the same function more than once. – crush Feb 04 '14 at 14:53
  • His line with the error shows him making the common mistake of calling his function instead of passing its reference. It doesn't show any indication that he confused passing a named function with passing an anonymous function. – crush Feb 04 '14 at 14:54
  • 1
    and you forgot to see the `{ });` ? – Loïc Faure-Lacroix Feb 04 '14 at 14:55
  • You're right. I did overlook that. It's still not clear that he wanted an anonymous function, but you might be right. There are still a lot of issues with your answer. Please clean those up. – crush Feb 04 '14 at 14:58
  • I've addressed the issues with my answer, now address the issues with yours. – crush Feb 04 '14 at 15:03
  • @crush anonymous could be faster as I doesn't have to do a name lookup. It will create a new function object on the fly which should be faster than searching the name inside closures but it really depends. – Loïc Faure-Lacroix Feb 04 '14 at 15:10
  • Do you have any [evidence to support that assertion](http://stackoverflow.com/questions/80802/does-use-of-anonymous-functions-affect-performance)? A function reference is a function reference is a function reference. ([also here](http://programmers.stackexchange.com/questions/193225/anonymous-function-performance-settimeout)) – crush Feb 04 '14 at 15:14
  • @crush I'll check what I can do. – Loïc Faure-Lacroix Feb 04 '14 at 15:19
  • I'm not a JavaScript developer. I'm used to PHP, .NET etc. I just wanted a function I could call to run the code. Just need to run the same code on the event as well as on load, so thought it would be more efficient making a function. – MRC Feb 04 '14 at 15:29
  • @crush my test were inconclusive in some sense.. the anonymous function was faster in a for loop up to around 15 cycles but the named function was way faster with more cycles. Adding more closures makes the named function loop slowlier but it's not enough to say it's a bad idea to use named function insead of anonymous. That said, if the function doesn't use variables in an outside closure the named function is much much faster. – Loïc Faure-Lacroix Feb 04 '14 at 16:07
  • Can you post your test for review? – crush Feb 04 '14 at 16:09
  • @Joey a function is fine. if you want to reference it multiple times anonymous functions are not the way to go. – Loïc Faure-Lacroix Feb 04 '14 at 16:09
  • @LoïcFaure-Lacroix I'm not even sure the difference, I guess the same as having private / public functions? – MRC Feb 04 '14 at 16:13
  • @LoïcFaure-Lacroix You could assign the reference of an anonymous function to a variable. That is what `var myFunction = function () {}` is doing, after all. Assigning the reference of an anonymous function to the variable `myFunction`. – crush Feb 04 '14 at 16:14
  • @crush check http://paste2.org/GIXsFXCC You can change the variable times to an other value to check how it affects both version. the variable `i` is used in both function as a closure variable. The only difference here is that doing array[0].somevar = "allo" isn't going to affect other indexes in the anonymous version. The named version is referenced on all indexes none of them will have their own properties in some sense. – Loïc Faure-Lacroix Feb 04 '14 at 16:20
  • @crush this gaves me the idea what would happen if we don't store the function anywhere. – Loïc Faure-Lacroix Feb 04 '14 at 16:21
  • @LoïcFaure-Lacroix On each iteration of the loop, a new instance of that anonymous function is created, while only a single instance of the named function is created. The named function should win hands down. Are you saying you observed the opposite? Try using jsperf.com or jsfiddle.com by the way. – crush Feb 04 '14 at 16:26
  • @crush change times to 10 for example. – Loïc Faure-Lacroix Feb 04 '14 at 16:27
  • @LoïcFaure-Lacroix I see what you mean, for very small iterations, anonymous is quicker; However, consider this: the difference is +/- 30ms at 5 iterations (in favor of anonymous), which is negligble. The difference is +/- 500ms at 1000 iterations (in favor of named). You should try one where you store a reference to an anonymous function in a variable. – crush Feb 04 '14 at 16:33
  • Wait, I just realized your named function is returning an anonymous function...why? – crush Feb 04 '14 at 16:34
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/46772/discussion-between-loic-faure-lacroix-and-crush) – Loïc Faure-Lacroix Feb 04 '14 at 16:38