Piano Player Solution

In the walkthrough video, min 7.55 it is explained that I cannot write assignment to the event as
note.onmousedown = keyPlay; but I have to call an assignment to an anonymous function note.onmousedown = function () {keyPlay(event)}; instead and I don’t fully grasp why the second method is correct and not the the first.
Any help is very appreciated. Thank you

  • H

The first way is valid depending on the context. The only difference between the two is the second way will be using a global variable called event while the first way** would expect an argument to be passed in.

** I know keyPlay isn’t shown here but just answered another question showing it had one param called event.

So the two code blocks are completely different, on takes an event when it is called and uses it, the other expects a variable called event to exist outside the function’s scope but still accessible, I said global above but technically doesn’t have to be depending on context.

Thank you jagking for your reply, my idea is to catch an event generated by the user.
I show you how I defined the keyPlay variable (which is a constant function in my case but it’s pretty much the same as the solution except it was not short-versioned).
The question was why the solution suggests (in the video) to implement a local anonymous function calling keyPlay (second case) on that event rather than calling keyPlay (my solution - first case). The video says I couldn’t’ do that and that is confusing to me.
The event is “raised” anyway, not using a global variable called event neither passing an event argument…

const keyPlay = (event) => {
event.target.style.backgroundColor = ‘green’;
}

keyPlay is the name of the callback function, a freestanding function that needs no parameter. event is a global variable.

written this way, we would expect the callback function to be coded anonymously, without a call to a standalone.

function (e) {
    // code statements
}

The parameter above takes the event object (aliased) which we can target…

note.onmousedown = function (e) {
    e.target.style.backgroundColor = 'green';
}
const keyPlay = () => {
    event.target.style.backgroundColor = ‘green’;
}

We only need this function if we do not write the callback anonymously, but rather assign the function reference to the listener.

1 Like

I was on my phone earlier to couldn’t do a full answer but to expanded on global Window.event:

You shouldn’t use the global event (Window.event). It originated in Internet Explorer, that should tell you all you need to know (even Microsoft consider ie a compatibility tool and don’t support it). In fact, when using shadow trees, (shadow DOMs are kind a big thing after all) it might not work properly. whatwg state that Window.event should be undefined with shadow trees and classify it as legacy. https://dom.spec.whatwg.org/#ref-for-dom-window-event

So if you want potential issues going forwards, keep doing it, otherwise just pass in the event to your function and know that it will always work. Also, it will probably, maybe, be deprecated one day…

If you have to use it, you’ll be working on some legacy internal app for business or government requiring ie 6 or something stupid, by which point you’ll know you need to use it.

As an aside, using the global like that is implicit and it isn’t going to help people read your code, especially inexperienced people, i.e. you in a week or two looking back at it. At least use Window.event.

The question was why the solution suggests (in the video) to implement a local anonymous function calling keyPlay (second case) on that event rather than calling keyPlay (my solution - first case). The video says I couldn’t’ do that and that is confusing to me.

The video is wrong if it said you can’t do that. The suggested solution of:

let keyAssignment = (note) => {
    note.onmousedown = function() {
        keyPlay(event);
    };
    note.onmouseup = function() {
        keyReturn(event);
    };
};

Is stupid too. Unless more logic is going to be added, why wrap keyPlay in another function, when you could just use it directly?

Talking of doing things the right way, I would do more like this:

const keyAssignment = (note) => {
    note.addEventListener('mousedown', keyPlay);
[... rest of code...]
}

And use note.removeEventListener to unregister the event once the element is removed from the page.
See:


and

For a full explanation of why they are better.

Good effort for questioning by the way.

1 Like

These courses use the global event listeners so there should not be a problem with using the global event object. There is little point in contradicting the course.

That being said, I have to say that only since coming to CC did I ever use global listeners (the ones with on prefix).

The addEventListener() method is a better choice, I would agree. However, it doesn’t hurt for a learner to come at events from a simple approach, which the globals offer. Probably the reason it was chosen for these units on events.

1 Like

Thank you for the explanations. As a Js beginner my question was about why the video says it’s not possible to use either way but only the anonymous function method ( https://youtu.be/0bbmEMAVQOo min 7.55).
Besides that, mtf, from my understanding I don’t even need to pass the event parameter to the function since events have global scope anyway (hence your last bit of code). Would it also work with the anonymous function solution?

1 Like

If we do this,

note.onmousedown = function (e) {
    e.target.style.backgroundColor = 'green';
}

then we need the parameter.

If we do this,

keyPlay = () => {

}
note.onmousedown = keyPlay

then no parameter, since there is no argument, only a function reference. The callback function will draw on the global event object to obtain a target element.

It was mentioned by @jagking that we should at least specify, Window.event so that we are explicitly addressing the global property, so we could write,

keyPlay = () => {
    Window.event.target.style.backgroundColor = "green"
}

The method given in the video is legitimate. We cannot assign a function call directly to the listener since it will execute immediately upon registering the listener and return undefined, so there will be no event handler. Hence, the anonymous function makes the call to keyPlay with an argument (could be e or event or fluffyBunny) as the name is only a parameter. Internally, the browser has only one active event at any one time, so only one event object.

eventAssignment = (note) => {
    on.mousedown = function (e) {
        keyPlay(e)
    }
    // ...
}

but, since there is an argument, our callback will need a parameter,

keyPlay = (event) => {
    event.target.style.backgroundColor = "green"
}

Again, here we name the parameter event but could have used any name. event makes sense, though, since it is the event object we are passing in.

There are two different approaches above. We cannot mix them. It’s one approach, or the other approach.

1 Like

I’m not saying the video is wrong, only that a solution I thought (sorry but I’m still learning) was still working but the video said I shouldn’t use it (and I don’t get why)
My code is:


// Write named functions that change the color of the keys below
const keyPlay = (event) => {
event.target.style.backgroundColor = ‘green’;
}

const keyReturn = (event) => {
event.target.style.backgroundColor = ‘’;
}

// Write a named function with event handler properties
let notesPlay = (note) => {
note.onmousedown = keyReturn;//function() {keyReturn(event)};
note.onmouseup = keyReturn;//function() {keyReturn(event)};
}

… with the solution suggested in the video commented //function() {keyReturn(event)};.
So I should follow the anonymous function path instead the one I came up with (but I found strange I shouldn’t since it’s working anyway).

Working with aliases and parameters is not a problem for me here and I undertood fully what you ment in those parts.
Thank you again

The use of explicit anonymous functions is favorable to callback references for that reason. I’ve never read anywhere that callback references are a bad thing or they would have been deprecated long ago. Personally, I don’t see any value in having an anonymous function calling another function when that function can carry out the actions right there.

note.onmousedown = (e) => {
    e.target.style.backgroundColor = "green"
}

I think that’s what @jagking was also suggesting. We’re really not breaking any rules, regardless which approach we use, especially just starting out. At some later time when all the beginner unawares have been gotten over and we begin to design optimized code models, best practices will become a concern. I wouldn’t set up too many hard and fast rules at this point.

1 Like

At a high level there is no difference between which method you choose, both will work. I’m going to pretend their method is:

note.onmousedown = (e) => {
    e.target.style.backgroundColor = "green"
}

The way they actually do it (calling the keyPlay in the lambda) is just code clutter and has no benefits.

The difference comes in the balancing act of not repeating yourself, self documenting code and not having cluttered code as well as performance.

Not repeating yourself
If you are using the function in more than one place then do your method. If you have to change it you only have to change it in one place is the main benefit.

Self documenting code
Giving something a name makes it easier to understand what it does. This isn’t too bad here because it is an event handler so clearly it is meant to handle the event. While you could technically name the function but nobody does that, they do anonymous functions. In another context, however see a name and not the function may help with readability.

Clean uncluttered code
This can go either way. Of you create the same function multiple times, it can clutter code. If you create a load of functions that are used once some may not like the look, equally lots of in place lambdas can make code look cluttered. This one is more subjective but there is a balance between the two methods.

Performance
This will depend on a number of factors, such as frameworks and browser JIT etc. But in react, for example using their method will create a new function each time a component rerenders, which isn’t very efficient.

1 Like