Blakeyrat's JavaScript Is Rusty Thread



  • Figured I'd just create one of these, since I'll likely have a lot of questions.

    First, I need someone to explain this:

    0_1499881930590_7225dcb1-3ab2-4d9d-b8f9-730be379dee2-image.png

    Buttons is a NodeList with zero elements. So why when I use for( x in y ) so I get the string "length"? Is JS thinking like, "well the collection's empty, so what he must want is the first property in the collection object itself?"

    So I have to do old-fashioned for( var i = 0; i < blah; i++ ) each time I have a NodeList? Someone tell me I'm crazy and there's a typo I'm not seeing.



  • @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    Buttons is a NodeList with zero elements. So why when I use for( x in y ) so I get the string "length"? Is JS thinking like, "well the collection's empty, so what he must want is the first property in the collection object itself?"

    for (x in y) is not iterating over collection, like in C#. It's iterating over keys of an object.

    var y = {a: 1, b: 2};
    for (var x in y) {
      console.log(x);
    }
    
    // 'a'
    // 'b'
    

    Array iteration is possible with arr.forEach(function (el) {...}), if you support ES5, I think.

    Since node list isn't an array, you'll have to cast it to one, or just do for (var i = 0; ...) crap.


  • FoxDev

    @blakeyrat No, you're right: for (x in y) will iterate through an object's properties. The only way to force it to iterate through the array as an array is for (var i = 0; i < a.length; i++).

    Of course, a JS expert will come along soon with about 5678392564839025784302 edge cases.



  • @raceprouk said in Blakeyrat's JavaScript Is Rusty Thread:

    The only way to force it to iterate through the array as an array is for (var i = 0; i < a.length; i++).

    Wow, that could not be less efficient with something like a NodeList, which is a perfect for an iterator. (If I only take the first 3 due to some logic, the browser still has to look at all 5737 to find the .length. Retarded.)


    Next question:

        this.installButtonHandlers = function()
        {
            var buttons = document.getElementsByTagName("button");
            for( var i=0; i< buttons.length; i++)
            {
                buttons[i].removeEventListener("click",this.eventHandler.bind(this));
                buttons[i].addEventListener("click",this.eventHandler.bind(this), true);
            }
        }
    

    If this function runs twice, I get two event listeners on every button. If it runs 3 times, I get three. Etc.

    How the fuck do I use removeEventListener with closures? It looks like what's happening is that DOM considers the first this.eventHandler.bind(this) as different from the second, and thinks I'm trying to remove a listener that isn't there.



  • [late to the party as my connection broke, but here's what I planned to say]

    Javascript for ... in ... object is fucked up (think has been fixed in later revisions to the spec.)

    It iterates over all properties in the object's prototype, including inherited ones such as 'length'.

    The accepted solution is usually to wrap loop body with a test to avoid inherited properties:

    e.g.

    for (var buttonIndex in buttons) {
      if (buttons.hasOwnProperty(buttonIndex)) {
        // do something or other with buttons[buttonIndex]
      }
    }

  • FoxDev

    @blakeyrat From the MDN JS docs:

    The bind() method creates a new function that, when called, has its this keyword set to the provided value, with a given sequence of arguments preceding any provided when the new function is called.

    (emphasis mine)

    Which of course means that the first this.eventHandler.bind(this) is different to the next this.eventHandler.bind(this).

    How to solve this? Honestly, I don't know. This is going to require someone with better JS knowledge than myself.



  • @raceprouk said in Blakeyrat's JavaScript Is Rusty Thread:

    Which of course means that the first this.eventHandler.bind(this) is different to the next this.eventHandler.bind(this).

    Well if this is the problem, I could create a global:

    this.boundHandler = this.eventHandler.bind(this);
    

    And pass that into addEventListener, I suppose. Really shitty "solution".

    EDIT: that moronic workaround from shitsville works, so moving on.



  • @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    So I have to do old-fashioned for( var i = 0; i < blah; i++ ) each time I have a NodeList? Someone tell me I'm crazy and there's a typo I'm not seeing.

    What others are saying is correct. for (var x in y) iterates over object keys, including those inherited from its prototype.
    You can use good old for (let i = 0, val = y[i]; i < y.length; ++i, val = y[i]) ..., or other methods suggested in this thread.
    With a sufficiently recent environment (ES6 or transpilers), you can also use for (const x of y) (of rather than in), which does exactly what you need.


    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    Next question:

        this.installButtonHandlers = function()
        {
            var buttons = document.getElementsByTagName("button");
            for( var i=0; i< buttons.length; i++)
            {
                buttons[i].removeEventListener("click",this.eventHandler.bind(this));
                buttons[i].addEventListener("click",this.eventHandler.bind(this), true);
            }
        }
    

    If this function runs twice, I get two event listeners on every button. If it runs 3 times, I get three. Etc.

    How the fuck do I use removeEventListener with closures? It looks like what's happening is that DOM considers the first this.eventHandler.bind(this) as different from the second, and thinks I'm trying to remove a listener that isn't there.

    Two functions only compare as equal if they are literally the same function. this.eventHandler.bind(this) creates a new function every time it executes, so those functions will never compare as equal and removeEventListener will never find any functions to remove. You can do this.boundEventHandler = this.eventHandler.bind(this) in the constructor, and then add/remove this.boundEventHandler as the listener.



  • just to add my two cents. because loops for... in loops are utterly broken, most js devs have started using arrays functions like .map and .reduce
    i don't know what browsers you have to support, but this site may prove to be useful.

    also, on the bind thing, your workaround seems about right, it's weird, but that's because you're in javascript



  • @blakeyrat I've always just done event handlers like:

    function someEventHandler(event) {
      //...
    }
    
    someElement.addEventListener('click', someEventHandler);
    

    and used the let _this = this; construct to avoid having to think about what Javascript will bind this to at any future time. If you do that you can .removeEventListener('click',someEventHandler) and it'll work the way you would expect. But if you really need the binding then maybe that won't work for you.



  • @hungrier Right; but I'm trying not to touch the global namespace at all with this code, so things become a bit more annoying all-around.

    There's another thread on that somewhere in the Help forums.



  • @blakeyrat you're looking for for ... of, not for ... in.

    If you're not able to use for ... of, then you have a few options...

    • iterate it the hard way
      for (var i = 0; i < buttons.length; ++ i) { ... }
    • make an array out of it, and use forEach
      [].slice.call(buttons).forEach(function (button) { ... })
    • borrow the forEach method from an array directly -- it doesn't know any better
      [].forEach.call(buttons, function (button) { ... })


  • @blakeyrat You don't need to touch anything global. Since your code is wrapped in an IIFE, everything declared with var or let should stay in that scope.



  • @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    It looks like what's happening is that DOM considers the first this.eventHandler.bind(this) as different from the second, and thinks I'm trying to remove a listener that isn't there.

    Well, yes. You created a new bound method when you called bind... the bound method created when you tell it to remove it isn't the same as the bound method created when you tell it to add it, so it can't be removed.

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    I could create a global:

    this.boundHandler = this.eventHandler.bind(this);
    

    Why not just bind this.eventHandler to this and be done with it?

    Actually, why are you binding this.eventHandler at all? And why's it a property of your this object? You're creating a closure, just to apparently leak everything that you create back out to the global scope... do you need it to be a global? That pretty much defeats the purpose of putting it inside a closure.

    Would this do what you're expecting?

    (function () {
    
    	function init()
    	{
    		var buttons = document.getElementsByTagName("button");
    		for (var i = 0; i < buttons.length; ++ i)
    		{
    			button.addEventListener("click", eventHandler);
    		}
    	}
    
    	function installButtonHandlers()
    	{
    		var buttons = document.getElementsByTagName("button");
    		for (var i = 0; i < buttons.length; ++ i)
    		{
    			buttons[i].removeEventListener("click", eventHandler);
    			buttons[i].addEventListener("click", eventHandler);
    		}
    	}
    
    	function eventHandler(event)
    	{
    		console.log("My button handler clicked");
    	}
    
    })();
    

    ...I mean, obviously you have to call those functions somewhere, but does it work?



  • @hungrier yeah, and his this reference is actually pointing back to window, so it seems to me that he's needlessly leaking everything back into the global scope.



  • @anotherusername Where? You'll have to elaborate on that, I don't see it.



  • @blakeyrat inside the closure. Unless you're binding it to something else before you're executing it, but I can't see that part of your code.

    var global_this = this;
    console.log(global_this); // logs the global object ("window" in browsers)
    
    // now execute a closure
    (function () {
        console.log(this); // still logs the global object
        console.log(this == global_this); // logs true
        this.method = function () { console.log("My method"); };
    })();
    
    console.log(method); // logs the method we created -- it leaked out of the closure
    method(); // runs, and logs "My method"
    

    0_1499892437856_688ab709-0c44-4388-a520-be12188f9f9b-image.png



  • @anotherusername Interesting, I think you're right. What's the easiest fix?



  • @blakeyrat To create private stuff inside a closure that doesn't leak out into the global scope, just define them as normal,

    (function () {
        var private = "stuff";
        function doStuff() { console.log("My private method"); }
    
        console.log(private);
        console.log(doStuff);
        doStuff();
    })();
    

    0_1499892771692_c1f9c190-1cc8-4a89-b8fc-b9d3c925cc21-image.png



  • @anotherusername Ah I get it. I was defining them that way because (if you go back to the old "update my JS style" thread) they were originally in a global object with a name and not a closure. But who knows, maybe my old project from 5 years ago was polluting the shit out of the global namespace too, now that I think about it.

    Thanks, I'll fix that right now.



  • @blakeyrat oh, forgot to add...

    A private method inside the closure can access a private variable also within the closure:

    0_1499893064634_e3981c43-61f3-401b-a08a-c31dcc38d95d-image.png

    And if you return a reference to a private method, then the private method will still be able to access the private variable -- it's running in the scope you defined it in, inside the closure:

    0_1499893104724_d44f32c4-1e66-4f29-9af5-89cceaf53d77-image.png

    ...which is handy, because you can encapsulate a whole bunch of shit, and just return a nice simple object with a couple of methods that can be called from the outside, while nothing else inside the closure leaks to the outside scope.



  • Cool, that's all fixed in both my affected JS files.



  • var input = searchParent.getElementsByTagName("input");
    
    if( input)
    {
                    
    }
    

    I assume getElementsByTagName returns something "false-y" if there are no elements, but since it always works in this case I'm not sure that's correct.

    Should I instead be checking .length == 0 ? Or will length be undefined if there's no elements in the NodeList?

    God I'm rusty at this shit.


  • Winner of the 2016 Presidential Election

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    Should I instead be checking .length == 0 ?

    ^This.



  • @asdf Yes because I just remembered those getElement(s)ByX return "live" objects that update in real-time as the DOM's shuffled around, so it wouldn't make sense for it to ever return a false-y value.

    This is how my brain works, it only pulls up the answer after I've asked the question.



  • @asdf or you could do it the javascript way,

    if (input.length) {
    
    }
    

    that'll skip 0 or null so you don't have to care!



  • @blakeyrat that's also a reason to consider using the array's slice method to make an array copy of the live collection.

    [].slice.call(arrayLike) will produce a static array out of arrayLike.


  • Banned

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    This is how my brain works, it only pulls up the answer after I've asked the question.

    You need a rubber duck.



  • @gąska Damn straight. Not used to working on projects 100% solo. Even when I've had "solo" code going on in the past, I was part of a team with co-workers and a stand up.

    It's taking some getting used to.



  • @anotherusername said in Blakeyrat's JavaScript Is Rusty Thread:

    that's also a reason to consider using the array's slice method to make an array copy of the live collection.

    It's a helper function to find one particular INPUT and grab it's .value. I get what you're saying, if I passed this NodeList around to other functions, but as-is nothing else has an opportunity to add/remove from DOM during its short lifetime.

    What's most frustrating is after killing all these old habits while learning C# LINQ-style, you go back to a more primitive programming environment and have to unlearn all of them. Ugh.

    Q: "Why am I looping though 6000 items just to find the count, just so I can do another loop to find the one I actually need which might be item #2 in the list?" (I mean, presumably DOM keeps track of the count in a slightly more sophisticated way, but still)
    A: "Welcome to 1988! Enjoy your ancient procedural code practices."


  • Impossible Mission - B

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    Q: "Why am I looping though 6000 items just to find the count, just so I can do another loop to find the one I actually need which might be item #2 in the list?"

    Hmm... how much work would it be to implement Take()?



  • @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    Why am I looping though 6000 items just to find the count

    Are you? Outside of enumerables (which javascript doesn't have baked in) and strlen (which is one mistake javascript somehow managed to avoid), getting the length of a collection is an O(1) operation.

    Also, since you hate primitive javascript, why not use typescript? (which I think you also had a thread about)
    It would allow you to use for-of (for arrays, at least), classes & lambdas (which don't have the 'this' problem), etc.
    It generally sits in the sweet spot between 'primitive' and 'too far from the target language'.

    Also also: at least your Rust isn't Javascripty!



  • @createdtodislikethis said in Blakeyrat's JavaScript Is Rusty Thread:

    Also, since you hate primitive javascript, why not use typescript? (which I think you also had a thread about)
    It would allow you to use for-of (for arrays, at least), classes & lambdas (which don't have the 'this' problem), etc.

    I thought about it, but this code requires really fast iteration time and I don't want a build system getting in my way. I'm actually having IIS just host the .js file directly out of the source control folder at the moment.


  • Impossible Mission - B

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    I'm actually having IIS just host the .js file directly out of the source control folder at the moment.

    :wtf:



  • @masonwheeler Kiss my ass, I'm iterating.



  • @createdtodislikethis I think what he's saying is that to loop a NodeList using the for (var i = 0; i < list.length; ++i) method, he thinks that under the hood it has to loop the whole NodeList at least once in order to find its length, before his loop can even begin to run.

    I don't know if it does or not. It might have some reasonably cheap way of keeping the length property up-to-date when the live NodeList changes.



  • @anotherusername Like storing the length in a field on the NodeList? (which it needs to do in order to allow application to index the NodeList properly anyway, this isn't c here)


  • Impossible Mission - B

    @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    I'm iterating.

    That's probably why it takes O(n) to get a Count. :trollface:


  • FoxDev

    @anotherusername I'd be surprised if it doesn't calculate the length as it builds the list



  • @raceprouk I'm sure there's a lot of efficiencies, considering how optimized DOM/JS is in general, but it's still stupid it can't build the list on-demand as I access bits of it.



  • @blakeyrat Ah, you mean you'd have wanted a lazy iterator instead of a list as the result from getElementsByWhatever. Yeah, that'd have been cleverer.



  • @anotherusername said in Blakeyrat's JavaScript Is Rusty Thread:

    @createdtodislikethis I think what he's saying is that to loop a NodeList using the for (var i = 0; i < list.length; ++i) method, he thinks that under the hood it has to loop the whole NodeList at least once in order to find its length, before his loop can even begin to run.

    I don't know if it does or not. It might have some reasonably cheap way of keeping the length property up-to-date when the live NodeList changes.

    avoid for (var i = 0; i < list.length; ++i) which will repeat the fairly expensive length calculation for each iteration of the loop, for (var i = 0, n = list.length; i < n; ++i) is faster.

    I can't think of many cases where the array slice alternative would be an efficient approach.



  • @createdtodislikethis I WANT MY LINQ GIVE ME MY LINQ!



  • @japonicus said in Blakeyrat's JavaScript Is Rusty Thread:

    avoid for (var i = 0; i < list.length; ++i) which will repeat the fairly expensive length calculation for each iteration of the loop,

    Will it though? I'm no Chromium developer, but if I was going to cache any value anywhere ever in DOM, it'd be that one.



  • @blakeyrat said in Blakeyrat's JavaScript Is Rusty Thread:

    @japonicus said in Blakeyrat's JavaScript Is Rusty Thread:

    avoid for (var i = 0; i < list.length; ++i) which will repeat the fairly expensive length calculation for each iteration of the loop,

    Will it though? I'm no Chromium developer, but if I was going to cache any value anywhere ever in DOM, it'd be that one.

    It certainly used to, but perhaps it's optimised now.



  • @createdtodislikethis said in Blakeyrat's JavaScript Is Rusty Thread:

    @blakeyrat Ah, you mean you'd have wanted a lazy iterator instead of a list as the result from getElementsByWhatever. Yeah, that'd have been cleverer.

    Blame IE.



  • @japonicus said in Blakeyrat's JavaScript Is Rusty Thread:

    Will it though? I'm no Chromium developer, but if I was going to cache any value anywhere ever in DOM, it'd be that one.

    It certainly used to, but perhaps it's optimised now.

    If you can mutate array during iteration, it's risky. If you have a hot loop, after many iterations maybe they can figure out they are safe to cache.



  • @cartman82 looks as though it doesn't

    https://jsperf.com/array-length-in-loop-dom

    but caching the length is probably well into irrelevant over-optimisation territory, so might not be worth worrying about (I'm probably too stuck in my ways).



  • @japonicus said in Blakeyrat's JavaScript Is Rusty Thread:

    @anotherusername said in Blakeyrat's JavaScript Is Rusty Thread:

    @createdtodislikethis I think what he's saying is that to loop a NodeList using the for (var i = 0; i < list.length; ++i) method, he thinks that under the hood it has to loop the whole NodeList at least once in order to find its length, before his loop can even begin to run.

    I don't know if it does or not. It might have some reasonably cheap way of keeping the length property up-to-date when the live NodeList changes.

    avoid for (var i = 0; i < list.length; ++i) which will repeat the fairly expensive length calculation for each iteration of the loop, for (var i = 0, n = list.length; i < n; ++i) is faster.

    I can't think of many cases where the array slice alternative would be an efficient approach.

    If order of iteration doesn't matter, you could always go backwards. for (var i = list.length - 1; i >= 0; --i) No need to worry about whether accessing .length is expensive because you only do it once.

    But yeah as said this is largely over-optimization.



  • @cartman82 said in Blakeyrat's JavaScript Is Rusty Thread:

    @createdtodislikethis said in Blakeyrat's JavaScript Is Rusty Thread:

    @blakeyrat Ah, you mean you'd have wanted a lazy iterator instead of a list as the result from getElementsByWhatever. Yeah, that'd have been cleverer.

    Blame IE.

    Is there any difference whatsoever between

    for (var value of list)

    and

    for (var value of list.values())

    ?

    Is there some way that list.values() works that's better? Because the example uses for...of, which works just as well with list as it does with list.values()... does it not?


Log in to reply