CODEWITHSUNDEEP
javascriptfunctional-programmingramda.js
bluehallu
bluehallu

Reputation: 10285

Can this Ramda.js data transformation be improved?

I have a list of stores and a list of ids:

var stores = [{id: '1', name: 'first store'}, {id: '2', name: 'second store'}, {id: '3', name: 'third store'}];
var ids = ['1', '2'];

I want to obtain the names of stores that match the ids on the list:

["first store", "second store"]

Here's what I came up with:

var filterStoresById = R.intersectionWith(R.useWith(R.equals, R.view(R.lensProp('id'))));
var extractNames =  R.map(R.view(R.lensProp('name')));
extractNames(filterStoresById(stores,ids));

I'm aiming to learn functional programming concepts while at the same time producing code I could use on real life but I don't think my solution is readable nor performant (I'm iterating twice), so what improvements could be made to this code?

Upvotes: 3

Views: 193

Answers (2)

Chris Vouga
Chris Vouga

Reputation: 554

innerJoin FTW

const stores = [
  {id: '1', name: 'first store'}, 
  {id: '2', name: 'second store'}, 
  {id: '3', name: 'third store'},
]

const ids = ['1', '2']

const f = 
    R.pipe(R.innerJoin(R.flip(R.propEq('id'))), R.pluck('name'))

console.log(f(stores, ids))
<script src="//cdnjs.cloudflare.com/ajax/libs/ramda/0.25.0/ramda.min.js"></script>

Upvotes: 1

Scott Sauyet
Scott Sauyet

Reputation: 50807

Your code is not horrible, only a little less readable than it might be.

First of all, to access a top-level property, especially when you're not reusing the access method, it's much simpler to use prop(name) than view(lensProp(name)). And Ramda has pluck for map(prop).

Second, I find it much easier to read functions built through functional composition than through nested levels of parentheses. I generally prefer pipe to compose, but either would do.

So I would rework your code like this:

var filterStoresById = R.intersectionWith(R.useWith(R.equals, R.prop('id')));
var storeNamesById = R.pipe(filterStoresById, R.pluck('name'));

storeNamesById(stores, ids);

One advantage to this is that you now have the reusable filterStoresById function if you find that you need more than just the names.

The other question is one of performance. There is definitely a penalty for iterating twice. The question is whether it is worth it for the cleaner, more easily refactored code. There are techniques for turning code like this into code that does the same thing but iterating only once and avoiding intermediate containers. You can see these articles on transducers for more information.

But I would avoid worrying about this unless you can demonstrate to yourself that this is actually a performance problem in your application. I assume everyone knows the Knuth misquote "premature optimization is the root of all evil."

Upvotes: 3

Related Questions