Fixing stuff: The REAL way



  • Sigh.

    Look at this shit:

    function forEach(callback, thisArg) {
    	thisArg = thisArg || thisDictionary;
    	var response,
    		index = 0;
    	for (var key in _data) {
    		response = callback.call(thisArg, key, _data[key], index);
    		index++;
    		if (response === false) {
    			return false;
    		}
    	}
    	return true;
    }
    

    thisDictionary is the custom dictionary class in a node.js project. _data is the backing data object.

    Notice anything wrong here?

    callback.call(thisArg, key, _data[key], index);

    IT'S FUCKING REVERSE

    Unlike every other forEach method ever, this one gives you the key, THEN the value in a callback.

    What FUCKING IDIOT thought this was a good ide-..

    ...

    Oh, right.

    It was me.

    Back when I was just starting node.js. Back when I though even having special data structures is a good idea in javascript. Back when I decided to plug this implementation into EVERY FUCKING class under the sun.

    So now, that I need to iterate over a collection in a generic way, I have to sniff whether the thing is a normal array or my fucking special snowflake dictionary.

    Ok, since I'm not going over the entire code base fixing this, I need a workaround.

    Surely, wiser programmers had met this problem. Sure they have come up with some pragmatic solution.

    Sigh.

    It won't be pretty... but I know what I have to do.


  • FoxDev

    async.each()?



  • ... is what I would do now, instead of making my own classes.


  • BINNED

    Next time on Fixing stuff: The REAL way:

    @cartman82 realizes his last fix is still wrong but it's now too widely used to get fixed.

    Watch him implement forEachRealHonest. Hilarity ensues.


  • FoxDev

    forEachRealHonestSeriouslyIMeanItThisTimeDefinitelyNoMoreOhJustOneMoreThisIsGettingRidiculousOhWhyDidntIJustUseAsyncInTheFirstPlace 😄




  • ♿ (Parody)

    forEachShitJustGotReal



  • I only became aware of Array.prototype.forEach relatively recently. Before that I was only aware of the jquery/angularjs features.

    Given that the first IE to support it was 9, I suspect it wouldn't have mattered. My last job included IE9 support.

    I'll gloss over the issues I have supporting MS's browser when I don't work for MS.


  • BINNED

    @Shoreline said:

    I only became aware of Array.prototype.forEach relatively recently. Before that I was only aware of the jquery/angularjs features.

    Ditto. And then I had another "oshit, have I been Doing It Wrong™ all this time?" moment when I saw whilst.

    Fortunately, as insane as they seem to be, people working on (ECMA|Java)Script haven't gone completely mental yet.

    It's just Node.js.



  • That only exists because ECMA is too chicken to put await in the language.



  • Just switch to using a forEach that's independent of the array being iterated, the same pattern as Angular and jQuery. Either use the one provided in the library you use, or create something that can be used like this:

    cartman82.forEach(myArray, function(arrayElement) {
      dostuff(arrayElement);
    });
    

    Your forEach could sniff if there is a native forEach implementation and call it - or provide its own.



  • @Jaime said:

    Just switch to using a forEach that's independent of the array being iterated, the same pattern as Angular and jQuery. Either use the one provided in the library you use, or create something that can be used like this:

    cartman82.forEach(myArray, function(arrayElement) {
    dostuff(arrayElement);
    });

    Your forEach could sniff if there is a native forEach implementation and call it - or provide its own.

    Yup, I started moving towards that pattern.



  • Un-f***ing code is always fun. It's like a dream within a dream...

    Is a dream within a dream still a thing or have we moved on?





  • @Frank said:

    It's like a dream within a dream...

    By any chance does this noise play in your head when you're un-fucking your code?

    https://www.youtube.com/watch?v=ZKGJZt83_JE



  • It's the sound of losing cash that launches me into un-fuck.

    It sounds like tin-snips going through carpet synchronized with the sound people make when their thumbnails gets slammed in a shutting car door.

    Either way it's reactive, electrical and highly motivating.



  • @cartman82 said:

    Back when I decided to plug this implementation into EVERY FUCKING class under the sun.

    I know fuckall about node but if you had to implement that in a bunch of code snippets why couldn't you make it into a library/helper class or whatever? So you only had to do it once...



  • @dstopia said:

    I know fuckall about node but if you had to implement that in a bunch of code snippets why couldn't you make it into a library/helper class or whatever? So you only had to do it once...

    I did make a Dictionary library. And then used that all over the place.


  • BINNED

    @Masaaki_Hosoi said:

    By any chance does this noise play in your head when you're un-fucking your code?

    Testing now. Doesn't really help with HTML / CSS though. Will retry when I'm back on backend stuff.



  • @cartman82 said:

    Unlike every other forEach method ever, this one gives you the key, THEN the value in a callback.

    @cartman82 said:

    I have to sniff whether the thing is a normal array or my fucking special snowflake dictionary.

    I was wondering why keyvaluepair is such a bad thing....

    then I noticed

    @cartman82 said:

    javascript

    I wonder if working out that bug is more work than just generating the types based on type-safe languages.

    Oh wait... you already created the types, otherwise you wouldn't have your "special snowflake".

    Well, there goes every argument for getting rid of type safety.



  • I wonder if working out that bug is more work than just generating the types based on type-safe languages.

    Yup, this kind of slip up is the best practical reason to prefer type safe languages. Everybody makes design mistakes. Type safety lets you fix them without running unit tests 3 dozen times.



  • Man, ever since linq and cascading transforms, I have all the flexibility without the risk.
    It just keeps getting better.



  • @Jaime said:

    cartman82.forEach(myArray, function(arrayElement) {
    dostuff(arrayElement);
    });

    @cartman82 said:

    Yup, I started moving towards that pattern.

    Humour me for a minute. Why not use Array.prototype.forEach?

    If you're wondering about object property iteration, I use:
    [code]
    Object.keys(cartmanObject).forEach(function (key) {
    var value = cartmanObject[key];
    });
    [/code]

    It's ugly, but I don't often iterate objects (probably because it's ugly).

    If the answer is because you didn't know about it, neither did I until a few months ago.

    The fun part is how I'm sufficiently indoctrinated into javascript that it doesn't actually bother me that much.

    Prototypal inheritance still bothers me. I just don't.



  • @Shoreline said:

    Humour me for a minute. Why not use Array.prototype.forEach?

    If you're wondering about object property iteration, I use:

    Object.keys(cartmanObject).forEach(function (key) {
    var value = cartmanObject[key];
    });

    It's ugly, but I don't often iterate objects (probably because it's ugly).

    If the answer is because you didn't know about it, neither did I until a few months ago.

    I knew about Array.forEach. I also know (and knew) how to iterate arrays and objects (btw doing Object.keys() thing is inefficient).

    Coming from C# background, I just thought that having a proper collection classes with restrictive public interfaces is the way to go. Unfortunately, all the advantages you get from this in C# (actual interfaces, intellisense) fail in javascript. Turns out, it's much better to have an external method library (eg. lodash) and manipulate native arrays or hashes. Live and learn.

    As for why I don't use this now, well my collections are doing something like:

    function Dictionary() {
    	var _data = {};
    
    	function set(key, value) {
    		_data[key] = value;
    	}
    	this.set = set;
    
    	//... etc
    }
    

    So I can only use the public api-s I have exposed, can't touch the data directly.

    This DOES have its advantages - see any OOP book for details. Except in this case I shot myself in the foot by having a standard method name with a non-standard interface, that I can't easily refactor (because the forEach name is used all over the place and the only way to refactor dynamic code is search & replace).


Log in to reply