6 ms·
I too prefer to use the "inefficient" notation because it just feels wrong (kind of an antipattern in the functional sense if you will) to mutate the argument.
by Rockslide 7y ago
I too prefer to use the "inefficient" notation because it just feels wrong (kind of an antipattern in the functional sense if you will) to mutate the argument. And as long as you are not dealing with a huge input list, this doesn't really matter at all.
Also, why would the memory complexity be n * n? Sure, a new object is created in each iteration, but its not like you have to keep the previous ones in memory - all but the last one can be garbage collected.
- minitech 7y agoIt’s n^2 time, not space. Each property of the object gets copied each time around, and the number of properties being copied increases each time around. It’s probably a good idea to start considering this kind of copying an antipattern. The good news is that the correct implementation doesn’t have to involve mutating any arguments, just a local with unambiguous ownership: const keyBy = (iterable, key) => { const map = new Map(); for (const value of iterable) { map.set(key(value), value); } return map; }; const peopleObj = keyBy(peopleArr, person => person.username); If you have an array, you even get to keep using array methods, which everyone loves: const keyBy = (array, key) => new Map( array.map(value => [key(value), value]) );
- c-smile 7y ago"new object is created in each iteration" Not just created, but properties are being populated one by one. "can be garbage collected" And what do you think is computational complexity of garbage collector? Yes, memory allocation in JS (GCable environment) is cheap. But garbage collection per se is not, it is at least O(N) complex.
- Rockslide 7y agoI never claimed garbage collection was free, but the point was memory complexity and not computational complexity. So claiming that the implementation would be O(n * n) in terms of memory complexity still doesn't make sense to me.
- megous 7y agoThere's nothing wrong with mutating an argument. The whole reason JS passes objects by reference is so that you can do it.
- Rockslide 7y agoI tend to treat my data structures as immutable, even when they technically aren't.
- bendiksolheim 7y agoThere are tons of wrongs with mutating arguments. The fact that it is possible to mutate arguments in JavaScript does not make it a good idea. Such functions cannot be trusted (how can you know what happens when you call it?), and they are hard to test. There is _always_ a better alternative to mutating arguments in a function.
- nitely 7y agoSure, but here it's a local mutation, it won't leak to the rest of the program. There is nothing wrong with mutating acc in reduce.
- tekkk 7y agoI agree, should you always test the reducing function separately outside the reduce? And if so, what's the real benefit of not mutating accumulator? Seems silly to just copy a transient value which is immediately discarded and then do it for all reduced elements. Does the original reference to the object even stay during the iterations? Hmm, I guess it does. To me it seems weird to be so puristic about a simple reduce-function, which by-design leads you to mutate the accumulator. I mean for object-accumulators it definitely is just a massive waste to not to just mutate the argument directly without copying. Although as a disclaimer I have to say I am big fan of simple code, were it FP or not. So if you have a messy reduce-function I guess having it immutable makes it a somewhat easier to manage.
- modarts 7y agoThis is a different argument than what megous was making (JS's support for mutating arguments validating that it's a good practice in a broad sense) "Unobservable" or local mutation is completely fine (and pretty common in most functional programs that get to any significant scale)