Real-World JavaScript Anti-Patterns | Hacker News Reader

Real-World JavaScript Anti-Patterns

blog.javascripting.com

I disagree on

    items.forEach(function(item) {  
        doSomething(item);
    });

being an anti pattern. Many of these looping functions actually pass more than one argument to the callback, so writing

    items.forEach(doSomething);

will break if doSomething is a function with optional arguments.

Yeah - not being aware of this will screw you up!

    ['10','10','10','10','10'].map(parseInt)
    // [10, NaN, 2, 3, 4]

Several "wat!"s on that line... I didn't know "10" was a number in base 0.

It gets even weirder:

    parseInt('10', {test: 'test'})
    10

    parseInt('10', 'test')
    10

    parseInt('10', '2')
    2

Indeed. The first version is actually more explicit in the sense that you know that only the `item` parameter is being passed to the function. The extra verbosity is worth it in my opinion.

It's true that in many cases the shorter version would not be harmful, but I have been bitten by a somewhat similar case where I meant for for the callback to be called with no arguments. Since the callback was passed in the latter style, it was called with arguments and the undesired behavior was difficult to debug. Or rather, it would have been, except that I work with incredibly smart people who saw the problem almost instantly.

This point-free programming style is good thing but requires knowing the exact method signatures. Luckily there are ways to easily adapt callbacks http://bahmutov.calepin.co/point-free-programming-is-not-poi...

Since this is a common occurrence, it might make sense to define a helper function to make it more natural, maybe something like:

    function only1(f) {
        return function(x) {
            return f(x);
        };
    }

then your example becomes:

    items.forEach(only1(doSomething));

Edit: just realized this is what "spots" in the blog post bahmutov linked to does (except spots is more flexible).

I'm not a Javascript developer, but I sort of disagree with the point about forEach and thisArg. Using a closure like that is clearer than relying on some custom magic done by a certain function.

Function.prototype.bind() makes sense, though.

Anti patterns? To what specific style of development? Imperative programming? functional? Class based / MVC / object oriented? Prototypical? This article lacks clarity and draws no useful conclusions.

"so one obvious improvement would be to use selectors instead of the execrable DOM API"

Additionally, DOM API is slow. Wrapping your access to the DOM in additional layers of abstraction isn't a win or "anti pattern". It's a maintenance trade off.

I just realized how I learned to put to good use many of the above stuff when using React. When I used to use Angular a common theme when I was writing my code was "How do I accomplish my goal by using Angular's API (as opposed to jQuery)?".

With React, since my "markup" and code is all just Javascript the common theme has become "How do I accomplish my goal in plain old Javascript?". The skills I've been picking up are definitely transferable to projects that don't use React or any other JS library.

Though I do keep forgetting to add `.bind(this)` all the time that it makes me miss using `var self = this;`.

Forgetting to use bind can be a good thing. See my comment above.

As for writing things "the Angular way", it's about truly separating your MVC layers using two way databinding. I don't care how you do it as long as you keep V as declarative as possible. It sounds like "the javascript way" for you might just involve a lot of storing data in the DOM.

I think a much more robust pattern involves:

A Model - This is Javascript data stored outside the DOM, for easy manipulation. It is usually accessible via the window object

A ViewModel - This is Javascript data that is stored outsidd the DOM but is referenced directly by a data-bound component. It can involve view transformations such as time/date localization.

A View - This is the actual DOM component, in Angular it is a "directive". It reacts to changes in the ViewModel and can also change the Model (which in turn changes the ViewModel).

That way, for example, if you want to implement optimistic visual feedback you can do it in the Model and have the model try to sync or revert.

All this becomes even more complex when you have to build multi user apps. You can use Firebass or Parse etc. or you can check out http://platform.qbix.com -- open sourced but still a work in progress until 1.0

> It sounds like "the javascript way" for you might just involve a lot of storing data in the DOM.

I don't think I've ever modified the DOM directly when using React. Try it out, it's a really refreshing take on building client-side apps.

The types of problems one becomes focused on are different between the two libraries - with Angular, the problem often is how to best architect an application. The lower level stuff are not as difficult once you familiarize yourself with Angular, although that admittedly has a high upfront cost of having to familiarize yourself with it.

React takes less opinions than Angular, thus leaving more for the user to determine how he/she wants to solve it. Fundamentally, React is more sound in that regard since it doesn't dominate everything as strongly - it is pretty clear that the Angular team agrees in this regard, and why they are moving to modularize everything in 2.0 (and why some formally core pieces such as the router were split off in 1.x).

Using either library helps with understanding of javascript and the browser - unless you contribute to/author angular components, React probably gives more precisely since it doesn't hide as much of the complexity. My own experience with Angular has been the deeper I delve into using it, the more I find myself moving towards using pure javascript in creating abstract classes while having the Angular-specific code be extensions of those classes - this pattern is transferable between frameworks/libraries, and likely an understated best practice when working in the browser.

While I agree with most of the points he listed, I have an issue with the style on which he presented them: In almost all cases, he presented the wrong solution, followed by an alternative which is "obviously" better - without explaining why it is better. The map-with-side-effects example is even worse, performance-wise than the original. Of course, performance tradeoffs of this kind are usually acceptable, but you usually want to know what you get in return. Especially in the for-each examples, this is not clear at all.

His second bind example is incorrect.

  function handleClick(i) {  
    this.innerHTML = i;
  }

  for (i = 0; i < elems.length; i++) {  
    elems[i].addEventListener("click", handleClick.bind(this, i));
  }

In the for loop, the this value refers to the global object. To get the desired effect, it should be changed to elems[i] (this piece of code is trying to bind a handler to an element).

I was wondering why the author mentioned _.partial after a sections about Function.prototype.bind. Wouldn't : _.partial(_.merge, {}).apply(this, confs); Be the same as: _.merge.bind(_, {}).apply(_, confs);

That being said, I'm glad this forced me to look up _.partial - seems like you can pass '_' as a parameter and that position in the arguments will not be bound in the partial function!

They should mention that most of the functions they use and recommend using don't exist in IE8 and crappier.

That's becoming less of an issue these days in general. But many of the examples use underscore/lodash, which do in fact support old IEs.

ES5shim pretty much takes care of that.

Most of them, especially the array methods, are present in every single ES5 shim in existence. There's no reason not to use ES5 shims if you have to work with IE8.

Some of these are good. Others, such as using .bind() just to create a handler for the click() event, are terrible. Not only are you creating a NEW closure every time the code runs (via executing .bind) but also you lose th ability to use the actual this inside the handler! Much better to use closures as they were intended, and have that extra _this var to refer to the previous context if necessary.

I disagree with the MyButton.prototype.new example - it isn't difficult to see that it is an api, and usage of new to do something like button.new() makes for very readable code.

There is good stuff here, but "anti-pattern" doesn't mean "you typed more than you really needed to". Rather simple tools can fix that sort of thing.

Using "this" is already an anti-pattern.

Only if you don't properly know JavaScript. Which is basically what this entire article is about.

Well JS is all about context, literally and figuratively I guess.

Why? I've heard people say that but have never heard a valid argument for using something else...

Reply on news.ycombinator.com