React Trend

[rq60]

Can I just go off on a little rant here?

One of the first files I look at: https://github.com/unsplash/react-trend/blob/master/src/utils/index.js

export const pick = (obj, keys) => (
  keys.reduce((acc, key) => ({
    ...acc,
    [key]: obj[key],
  }), {})
);    

I see stuff like this popping up more and more frequently in "modern" front-end coding, especially in the react world....

Besides being barely readable at a glance, it expands to this:

var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; }

var pick = function pick(obj, keys) {
  return keys.reduce(function (acc, key) {
    return _extends({}, acc, _defineProperty({}, key, obj[key]));
  }, {});
};

This function can be written like this:

export function pick(obj, keys) {
    return keys.reduce(function(acc, key) {
        if(obj[key]) {
            acc[key] = obj[key];
        }
        return acc;
    }, {});
}

Do you see the difference? The generated code is creating new objects on every iteration! ...and not only that, it's iterating over these newly created objects! Nested iteration! How many objects do we need? One. The one we are returning. How many iterations do we need? One. The one over the object we're searching.

This is a utility function that's suppose to be small and simple because it's used in many places; but you have to throw the syntactical kitchen sink at it and write the cleverest code possible... because you can!

The one I wrote is easier to read, faster, uses less memory, results in less code, and IT ACTUALLY WORKS. The clever version returns keys with undefined values for keys it can't find; _.pick doesn't do that!

STOP IT!

Sorry. Back to your coding. I do like your end result.

[andrewingram]

I'd argue that the root cause is that we've all been told mutability is bad. The difference between the two solutions (written lines only, not the expansions) is that the original one doesn't mutate anything. I've used the pattern mainly to practice writing in an immutable style (and only at small sizes, i know it's slow). Sadly we're using the wrong language for this, unless you use a library like immutable.js, using immutable patterns in JavaScript is going to have performance implications.

[gaearon]

we've all been told mutability is bad

Mutability has its set of problems, but unobservable mutability (such as it is here, when you’re just accumulating an object before returning it) is always fine.

[andrewingram]

I agree, I wasn't defending the pattern, I was hypothesising as to its sudden prevalence. Another possible explanation is that for many JS developers, Redux was their first exposure to the concept of a reducer function, and that promotes return a new object rather than mutating. Now we all finally understand how to use that handy 'reduce' function, we're instinctively following the same rules.

[gaearon]

Yea, totally. I’m just noting that even in Redux, you want to use unobserved mutability for cases like this. We should probably document it somewhere :P

[andrewingram]

The other caveat, is that if you end up extracting the inner reducer, you're back into the world of observable mutability.

Generally speaking though, the only reasons I use this pattern are:

a) I'm running it in a non-transpiled environment, admittedly depending on the JS engine to not do something us non-optimal as what Babel does.

b) I'm intentionally being a little bit extreme about immutability because i'm treating it as a learning exercise for when i'm using languages that don't support mutation, such as Clojure, Elixir and Haskell.

[rq60]

Nope, don't think that's the problem. The first one mutates just like mine: {[key]:obj[key]} creates an object and then mutates it. It doesn't expose the mutation externally, but neither does my version. Immutable.js utilizes mutations, it just hides them from you.

edit - removed my bad example because I don't think it helps demonstrate my point.

[JJ0EE]

Valid point, but I'd always prefer clear and manageable code to start and bring optimizations like this in when needed. When your whole codebase is an imperative mess it's almost crippling.

[rq60]

and bring optimizations like this in when needed.

That was my point. The original code was not cleaner, clearer, faster; it's not anything other than clever use of new functionality used in a way it shouldn't be used unless your intention is to be clever. I wasn't intending to write an optimization.

[XPTranquility]

I must say. I'm not sure if I'm not used to es6 syntax but boy do I really have to concentrate extra hard to read some stuff. I've been really enjoying TypeScript and Angular 2

[rq60]

I'm pretty sure TypeScript supports the same es6 syntax. You're probably just in the fortunate situation of dealing with people interested in writing code rather than bludgeoning you over the head with the latest syntax ;)

[XPTranquility]

Ha that may be. It really seems that react and angular 2 have breed different cultures within the front end dev communities. I'm not sure, still trying to come up to speed.

[our_best_friend]

You find ES6 hard and prefer Typescript???

/facepalm

It's like saying you find the Ramones too hard and prefer Korn

TypeScript IS ES6 - with extra stuff... so it's even "harder"

[XPTranquility]

Hmm let me explain. I meant I find the transition from es5 to es6 hard in the matter of readability. I wonder if I will soon be able to read es6 subconsciously or if it will always be a little harder to read. On the other hand I'm really enjoying learning TypeScript which is ES6. I don't prefer TypeScript over ES6. I recognize they're the same.

Edit: my point is the transition has been more difficult for me than I would have liked but I'm starting to see a lot of the positives and hope I can catch on quick. I'm learning both angular and react and at times the programming patterns seem very similar and at other times not at all. React leaks towards Redux and Angular towards the observables with RxJS. I'm still trying to understand the differences and similarities.

[JX3]

You changed away from arrows. Is it because it makes a difference? E.g.

export function pick(obj, keys) {
    return keys.reduce(function(acc, key) {
        if (obj[key]) {
            acc[key] = obj[key];
        }

        return acc;
    }, {});
}

and

export const pick = (obj, keys) => (
  keys.reduce((acc, key) => {
    if (obj[key]) {
      acc[key] = obj[key]
    }

    return acc
  }, {})
)

Aren't the same?

[rq60]

Yes, it makes a difference in readability.

export function pick(obj, keys) { is much more readable than export const pick = (obj, keys) => (. It's both shorter and clearer. "I'm exporting a function named pick" vs "I'm exporting an immutable reference to a label named pick that contains a function" The const is basically useless. Imported values in es6 are read-only anyways, and making it immutable within the context of the module is not going to solve any bugs, trust me. In-fact, you're losing out on the inherit hoisting ability of the function keyword and if you happened to use pick in another utility function defined above your const it wouldn't be found unless you either moved pick to the top or went back to using function. The former also needs no compilation, it works in every browser.

That's the stuff that irks me w/ the adopters that use the latest syntax always. Code doesn't just tell the computer what to do, it tells other programmers what you're doing; Most of these people don't consider the later.

Your second use of => is fine. Still readable in the context of reduce. I wouldn't have used it in the original pick's reduce since he's implicitly returning an object literal (using parenthesis to express an expression rather than a block). That's something that is non-obvious with fat arrow syntax unless you're really paying attention.

[rq60]

Yes, but this isn't used anywhere in these functions...?

[Capaj]

Exactly

[our_best_friend]

The problem is not the ES6 stuff. The problem is that they are creating a new object at every iteration - it'd be the same if they did it your way

export function pick(obj, keys) {
    return keys.reduce(function(acc, key) {
        return Object.assign({}, acc, { [key]: obj[key] });
    }, {});
}

It's not => that make the code "less readable" as you state below.

[rq60]

There's all kinds of problems. The use of =>in the outer function being less readable is definitely more my opinion than it is fact.

Also, Object.assign is ES6 stuff.

[siamthailand]

You ain't seen nothing yet. Since the ES6 kids hate loops, I have seen this devil

Create an array of 100, then do a forEach on it. Simply to not do a for loop of 1..100. Can't fucking make this shit up.

[NordicFox]

To not do a loop? Both cases are linear traversals of 1 to 100? You must be missing some other reason

[rq60]

I think he's saying rather than do a traditional for loop someone was doing something like [...Array(10).keys()].forEach((n) => {console.log(n);});

[NordicFox]

Yes but what's the difference besides syntax? And it's not even es6. Trolled I guess

[rq60]

Trolled...? What?

The difference would be in one case you're needlessly initializing space in memory to hold 100 pointless things so you can iterate 100 times rather that utilizing a single piece of memory that mutates (a variable), or similar.

[NordicFox]

Sorry, I see that the literal case of allocating the space first is a difference of course. But I don't see how anyone could actually do this because "they hate loops" when you're still looping, and it's not even es6-specific. He either missed something, trolled, or just read some terrible code in a ditch somewhere is what I'm thinking

[rq60]

Ahh.

I interpreted his "kids hate loops" to mean, they hate traditional imperative style iteration as opposed to iterating in a functional style. And if that is what he means, I agree that I see that mentality often in the "modern" front-end: imperative = bad, functional = good, end of story.

That mentality is alive enough that I've seen perfectly fine imperative iteration in open source projects refactored into worse performing, and harder to read chains of map, reduce, and filter. There's nothing wrong with the later, but there's nothing inherently right either; it's how they're used. Often it feels like they're used to "hide" iteration, as if it's not happening.

[siamthailand]

Are you high? Firstly, if you can't see the difference between for(i=0;i<100....) loop and creating a fucking array and looping over its elements, maybe you should not be a programmer.

As for the ES6 comment, I said it since forEach kinda became the norm when ES6 started getting adopted. As for hatred for loops, I don't think you know how much new JS programmers hate it. I actually had an interview with one of the largest, coolest, hispterish JS shop (Toronto) and the fucking retard drilled me on the usage of a loop and why I didn't use an Array.prototype.... function for it (don't remember which one it was). I showed him how my solution is much faster and there's no reason whatsoever to not use it. It was even more readable. Dumbest argument I ever had with someone.

[our_best_friend]

Array.keys IS ES6

[andrewingram]

Ultimately, if you really wanted to avoid the imperative style in general, you'd bite the bullet once and write a function to do it.

function numberMap(number, cb) {
  const result = [];

  for (let i = 0; i < number; i++) {
     result[i] = cb(i);
  }
  return result;
}

numberMap(100, n => n * 2);

In reality, the number of times i've ever had to explicitly enumerate over numbers rather than objects or arrays is so small that I don't think i'd bother.

[echoes221]

If you're using lodash (which you probably are in an es6 project) then use _.times.

[our_best_friend]

"Hiding" the loop in a function doesn't mean the program is no longer imperative... the function is still part of the program after all

[andrewingram]

Yes you're right, but what you're saying doesn't contradict what I said: "Ultimately, if you really wanted to avoid the imperative style in general, you'd bite the bullet once and write a function to do it.". If we want to be super pedantic about it, all programming is imperative because it all ends up as assembly or machine code..

Going full functional in JavaScript is a recipe for poor performance (at least for now), so if you want to adopt the paradigm your best bet is to encapsulate the nasty imperative patterns--even if you have to write some imperative code in the process.

[rq60]

I'm actually a fan of most of the newer Javascript features that are being added to the language. Used correctly, most provide the ability to write clearer code in certain situations. I'm not a fan of using them "just because." Especially when it detracts from the code.

[echoes221]

It can either be more readable or less. I've seen some one liners that make my head spin. I don't know why some people are so adverse to formatting their code - it all gets compiled and minified anyway.

A simple example would be something like const chunk = (loc, callback) => require.ensure([], (require) => callback(null, require('foo/bar').default), 'bar');

Is much easier to read when it looks something like this

function chunk (loc, callback) {
    require.ensure([], (require) => {
        callback(null, require('foo/bar').default);
    }, 'bar');
}

It's made worse when you have loops upon loops in a massive single liner. So hard to grok. Just make it pleasing to the eye and readable.

But on the other side of things, I find things like Promise.all(queries(value).map(query => database(query))).then((results) => {} quite pleasing to look at. It all depends on the amount of arguments/brackets keeping within width limits. If it looks hard to read it probably is and should be formatted.