5
\$\begingroup\$

If I use node.addEventListener('click',Wyg.Editor.nodeClickedEvent); then when node is clicked, nodeClickedEvent has this===node. Since nodeClickedEvent is a static function in a class, I want this to === Wyg.Editor, so I wrote a helper function to simplify it.

Helper Function:

function fixThisFunc(object,functionName){
    const fixedFunc = function(){
        object[functionName].apply(object,arguments);
    };
    return fixedFunc;
}

How it's used:

let node = document.getElementById('myFavoriteNode');
node.addEventListener('click', fixThisFunc(Wyg.Editor,'nodeClickedEvent'));

How it looked before:

node.addEventListener('click',
    function(event){
        Wyg.Editor.nodeClickedEvent(event);
    }
);

The Wyg.Editor class is like:

Wyg.Editor = class {

    static nodeClickedEvent(event){
        const clickedNode = event.target;
        console.log(this); //successfully outputs the Wyg.Editor object/class
        this.getEditableNode(clickedNode);
    }
//there are more functions, of course
};

I'm looking for a review of the Helper Function fixThisFunc

Are there any particular problems I should be worried about? is there a better way to achieve this functionality?

\$\endgroup\$
  • \$\begingroup\$ Do you mean to use this for this particular scenario or as an API reusable across use cases? \$\endgroup\$ – dfhwze Sep 17 at 20:16
  • 1
    \$\begingroup\$ isn't bind enough?\ \$\endgroup\$ – Semi-Friends Sep 18 at 7:55
  • \$\begingroup\$ @Semi-Friends, yes. Yes it is! Didn't know about it. \$\endgroup\$ – Reed Sep 18 at 20:13
6
\$\begingroup\$

is there a better way to achieve this functionality?

You can use Function.bind() to create a function with the this context bound to Wyg.Editor:

const node = document.getElementById('myFavoriteNode');
node.addEventListener('click', Wyg.Editor.nodeClickedEvent.bind(Wyg.Editor));

See this demonstrated in the snippet below.

Notice that const was used instead of let - unless there is a reason to re-assign node, use const. This will avoid accidental re-assignment.

const Wyg = {};
Wyg.Editor = class {

    static nodeClickedEvent(event){
        const clickedNode = event.target;
        console.log('nodeClickedEvent() - this: ', this); 
        
    }
};
const node = document.getElementById('myFavoriteNode');
node.addEventListener('click', Wyg.Editor.nodeClickedEvent.bind(Wyg.Editor));
<button id="myFavoriteNode">click this favorite node</button>

\$\endgroup\$
  • \$\begingroup\$ Thanks! That's exactly what I need. \$\endgroup\$ – Reed Sep 18 at 20:14
4
\$\begingroup\$

Accessing statics via this

Defining the object reference outside the object just to gain access via a miss used accessor (this) is a hack and not how to use static objects to handle events.

Binding objects to a function

First what you did could have been a little less complicated

You had something like

class Editor {
    static nodeClick(event){
        this.editable(event.target);  // miss used 'this' to reference 'Editor'
    }
    static editable() {} 
}


function fixThis(object,functionName){
    const fixedFunc = function(){
        object[functionName].apply(object, arguments);
    };
    return fixedFunc;
}
node.addEventListener('click', fixThis(Editor,'nodeClick'));

Could have written the binding using Function.call

const  thisFunc = (obj, func) => (...args) => func.call(obj, ...args);
node.addEventListener('click', fixThis(Editor, Editor.nodeClick));

or using Function.bind as one line

node.addEventListener('click', Editor.nodeClick.bind(Editor));

Access static via name

Static functions should access properties via the defined name. This makes it clear that you are accessing the static properties and conforms with static property accessed from within an instance of the object.

Your objects static access via name should look like

class Editor {
    static nodeClick(event){
        Editor.editable(event.target);  // correct reference to 'Editor'
    }
    static editable() {}
}

Then you don't need to bind Editor to Editor.nodeClick to maintain the correct reference. Just pass the static function as is, to the event

node.addeventListener("click", Editor.nodeClick);
\$\endgroup\$
  • \$\begingroup\$ I'll be using bind. I find that produces a much more predictable result, and I like it. Hacky or not. I don't want to hard code class names, because that makes my code more dependent upon my naming practices. Using this means I can refactor & rename things and still get the same result. \$\endgroup\$ – Reed Sep 18 at 20:24
2
\$\begingroup\$

is there a better way to achieve this functionality?

You might use a wrapping function:

let node = document.getElementById('myFavoriteNode');
node.addEventListener('click', event => Wyg.Editor.nodeClickedEvent(event));

// or, using regular function expression
node.addEventListener('click', function(event) {
  return Wyg.Editor.nodeClickedEvent(event);
});

Using a wrapper function lets you keep the original context of your nodeClickedEvent method the way you expect. You can use either arrow or regular function expression, because you don't care at all about the this value addEventListener provides to the callback.

\$\endgroup\$
  • \$\begingroup\$ The event => ... is far more concise than what I originally had, and I do kind of like it. But I'm gonna roll with bind. Fits how I like to code, because I'd rather use this wherever I can than use a hard-coded class name. \$\endgroup\$ – Reed Sep 18 at 20:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.