1

I am trying to switch my programming style to declarative from imperative, but there is some concept that is bugging me like the performance when it comes to the loop. For example, I have an original DATA, and after manipulating it I wish to get 3 expected outcomes: itemsHash, namesHash, rangeItemsHash

// original data

const DATA = [
  {id: 1, name: 'Alan', date: '2021-01-01', age: 0},
  {id: 2, name: 'Ben', date: '1980-02-02', age: 41},
  {id: 3, name: 'Clara', date: '1959-03-03', age: 61},
]

...

// expected outcome

// itemsHash => {
//   1: {id: 1, name: 'Alan', date: '2021-01-01', age: 0},
//   2: {id: 2, name: 'Ben', date: '1980-02-02', age: 41},
//   3: {id: 3, name: 'Clara', date: '1959-03-03', age: 61},
// }

// namesHash => {1: 'Alan', 2: 'Ben', 3: 'Clara'}

// rangeItemsHash => {
//   minor: [{id: 1, name: 'Alan', date: '2021-01-01', age: 0}],
//   junior: [{id: 2, name: 'Ben', date: '1980-02-02', age: 41}],
//   senior: [{id: 3, name: 'Clara', date: '1959-03-03', age: 61}],
// }
// imperative way

const itemsHash = {}
const namesHash = {}
const rangeItemsHash = {}

DATA.forEach(person => {
  itemsHash[person.id] = person;
  namesHash[person.id] = person.name;
  if (person.age > 60){
    if (typeof rangeItemsHash['senior'] === 'undefined'){
      rangeItemsHash['senior'] = []
    }
    rangeItemsHash['senior'].push(person)
  }
  else if (person.age > 21){
    if (typeof rangeItemsHash['junior'] === 'undefined'){
      rangeItemsHash['junior'] = []
    }
    rangeItemsHash['junior'].push(person)
  }
  else {
    if (typeof rangeItemsHash['minor'] === 'undefined'){
      rangeItemsHash['minor'] = []
    }
    rangeItemsHash['minor'].push(person)
  }
})
// declarative way

const itemsHash = R.indexBy(R.prop('id'))(DATA);
const namesHash = R.compose(R.map(R.prop('name')),R.indexBy(R.prop('id')))(DATA);

const gt21 = R.gt(R.__, 21);
const lt60 = R.lte(R.__, 60);
const isMinor = R.lt(R.__, 21);
const isJunior = R.both(gt21, lt60);
const isSenior = R.gt(R.__, 60);


const groups = {minor: isMinor, junior: isJunior, senior: isSenior };

const rangeItemsHash = R.map((method => R.filter(R.compose(method, R.prop('age')))(DATA)))(groups)

To achieve the expected outcome, imperative only loops once while declarative loops at least 3 times(itemsHash,namesHash ,rangeItemsHash ). Which one is better? Is there any trade-off on performance?

dummy
  • 21
  • 6
  • 2
    "*To achieve the expected outcome, imperative only loops once while declarative loops several times.*" uh, because ***you*** have made the declarative code to loop multiple times. You can group with a single loop over a dataset. In Ramda, that's `groupBy`/`groupWith` – VLAZ Apr 07 '21 at 09:40
  • 1, Is it possible for you to show me the optimized code to get those 3 outcomes? 2, ```groupBy``` maybe fits in this example, but when it comes to more complex scenarios, what to do then? – dummy Apr 07 '21 at 09:49
  • I mean to achieve ```rangeItemsHash ```, 1 single loop of```groupBy``` does the trick. But what about the other two ``itemsHash ``, ```namesHash ```? Do not they take two lines of the loop? – dummy Apr 07 '21 at 09:55

2 Answers2

1

I have several responses to this.

First, have you tested to know that performance is a problem? Far too much performance work is done on code that is not even close to being a bottleneck in an application. This often happens at the expense of code simplicity and clarity. So my usual rule is to write the simple and obvious code first, trying not to be stupid about performance, but never worrying overmuch about it. Then, if my application is unacceptably slow, benchmark it to find what parts are causing the largest issues, then optimize those. I've rarely had those places be the equivalent of looping three times rather than one. But of course it could happen.

If it does, and you really need to do this in a single loop, it's not terribly difficult to do this on top of a reduce call. We could write something like this:

// helper function
const ageGroup = ({age}) => age > 60 ? 'senior' : age > 21 ? 'junior' : 'minor'

// main function
const convert = (people) =>
  people.reduce (({itemsHash, namesHash , rangeItemsHash}, person, _, __, group = ageGroup (person)) => ({
    itemsHash: {...itemsHash, [person .id]: person},
    namesHash: {...namesHash, [person .id]: person.name},
    rangeItemsHash: {...rangeItemsHash, [group]: [...(rangeItemsHash [group] || []), person]}
  }), {itemsHash: {}, namesHash: {}, rangeItemsHash: {}})

// sample data
const data = [{id: 1, name: 'Alan', date: '2021-01-01', age: 0}, {id: 2, name: 'Ben', date: '1980-02-02', age: 41}, {id: 3, name: 'Clara', date: '1959-03-03', age: 61}]

// demo
console .log (JSON .stringify (
  convert (data)
, null, 4))
.as-console-wrapper {max-height: 100% !important; top: 0}

(You can remove the JSON .stringify call to demonstrate that the references are shared between the various output hashes.)

There are two directions I might go from here to clean up this code.

The first would be to use Ramda. It has some functions that would help simplify a few things here. Using R.reduce, we could eliminate the annoying placeholder parameters that I use to allow me to add the default parameter group to the reduce signature, and maintain expressions-over-statements style coding. (We could alternatively do something with R.call.) And using evolve together with functions like assoc and over, we can make this more declarative like this:

// helper function
const ageGroup = ({age}) => age > 60 ? 'senior' : age > 21 ? 'junior' : 'minor'

// main function
const convert = (people) =>
  reduce (
    (acc, person, group = ageGroup (person)) => evolve ({
      itemsHash: assoc (person.id, person),
      namesHash: assoc (person.id, person.name),
      rangeItemsHash: over (lensProp (group), append (person))
    }) (acc), {itemsHash: {}, namesHash: {}, rangeItemsHash: {minor: [], junior: [], senior: []}}, 
    people
  )

// sample data
const data = [{id: 1, name: 'Alan', date: '2021-01-01', age: 0}, {id: 2, name: 'Ben', date: '1980-02-02', age: 41}, {id: 3, name: 'Clara', date: '1959-03-03', age: 61}]


// demo
console .log (JSON .stringify (
  convert (data)
, null, 4))
.as-console-wrapper {max-height: 100% !important; top: 0}
<script src="//cdnjs.cloudflare.com/ajax/libs/ramda/0.27.1/ramda.js"></script>
<script> const {reduce, evolve, assoc, over, lensProp, append} = R   </script>

A slight downside to this version over the previous one is the need to predefine the categories senior, junior, and minor in the accumulator. We could certainly write an alternative to lensProp that somehow deals with default values, but that would take us further afield.

The other direction I might go is to note that there is still one potentially serious performance problem in the code, one Rich Snapp called the reduce ({...spread}) anti-pattern. To solve that, we might want to mutate our accumulator object in the reduce callback. Ramda -- by its very philosophic nature -- will not help you with this. But we can define some helper functions that will clean our code up at the same time we address this issue, with something like this:

// utility functions
const push = (x, xs) => ((xs .push (x)), x)
const put = (k, v, o) => ((o[k] = v), o)
const appendTo = (k, v, o) => put (k, push (v, o[k] || []), o)

// helper function
const ageGroup = ({age}) => age > 60 ? 'senior' : age > 21 ? 'junior' : 'minor'

// main function
const convert = (people) =>
  people.reduce (({itemsHash, namesHash , rangeItemsHash}, person, _, __, group = ageGroup(person)) => ({
    itemsHash: put (person.id, person, itemsHash),
    namesHash: put (person.id, person.name, namesHash),
    rangeItemsHash: appendTo (group, person, rangeItemsHash)
  }), {itemsHash: {}, namesHash: {}, rangeItemsHash: {}})

// sample data
const data = [{id: 1, name: 'Alan', date: '2021-01-01', age: 0}, {id: 2, name: 'Ben', date: '1980-02-02', age: 41}, {id: 3, name: 'Clara', date: '1959-03-03', age: 61}]

// demo
console .log (JSON .stringify (
  convert (data)
, null, 4))
.as-console-wrapper {max-height: 100% !important; top: 0}

But in the end, as already suggested, I would not do this unless performance was provably a problem. I think it's much nicer with Ramda code like this:

const ageGroup = ({age}) => age > 60 ? 'senior' : age > 21 ? 'junior' : 'minor'

const convert = applySpec ({
  itemsHash: indexBy (prop ('id')),
  nameHash: compose (fromPairs, map (props (['id', 'name']))),
  rangeItemsHash: groupBy (ageGroup)
})

const data = [{id: 1, name: 'Alan', date: '2021-01-01', age: 0}, {id: 2, name: 'Ben', date: '1980-02-02', age: 41}, {id: 3, name: 'Clara', date: '1959-03-03', age: 61}]

console .log (JSON .stringify(
  convert (data)
, null, 4))
.as-console-wrapper {max-height: 100% !important; top: 0}
<script src="//cdnjs.cloudflare.com/ajax/libs/ramda/0.27.1/ramda.js"></script>
<script> const {applySpec, indexBy, prop, compose, fromPairs, map, props, groupBy} = R </script>

Here we might want -- for consistency's sake -- to make ageGroup point-free and/or inline it in the main function. That's not hard, and another answer gave an example of that. I personally find it more readable like this. (There's also probably a cleaner version of namesHash, but I'm out of time.)

This version loops three times, exactly what you are worried about. There are times when that might be a problem. But I wouldn't spend much effort on that unless it's a demonstrable problem. Clean code is a useful goal on its own.

Scott Sauyet
  • 49,207
  • 4
  • 49
  • 103
  • Wow, much thanks to you, sir. See, I know it is not a very big deal worrying about a fancy performance problem too much. The main point here is for me to ask about what is the most recommended way to achieve my goal in this example. And I think I got the answer from you. – dummy Apr 08 '21 at 02:06
  • Another question, _"So my usual rule is to write the simple and obvious code first"_, don't you think my first part code is simple enough? Would you prefer doing that instead of writing it in Ramda approach? – dummy Apr 08 '21 at 02:11
  • @dummy: To me, my final sample is significantly simpler than either of yours. But as one of the founders of Ramda, I do tend to think in terms of its capabilities. YMMV – Scott Sauyet Apr 08 '21 at 04:05
0

Similar how to .map(f).map(g) == .map(compose(g, f)), you can compose reducers to ensure a single pass gives you all results.

Writing declarative code does not really have anything to do with the decision to loop once or multiple times.

// Reducer logic for all 3 values you're interested in
// id: person
const idIndexReducer = (idIndex, p) => 
  ({ ...idIndex, [p.id]: p });

// id: name
const idNameIndexReducer = (idNameIndex, p) => 
  ({ ...idNameIndex, [p.id]: p.name });
  
// Age
const ageLabel = ({ age }) => age > 60 ? "senior" : age > 40 ? "medior" : "junior";
const ageGroupReducer = (ageGroups, p) => {
  const ageKey = ageLabel(p);
  
  return {
    ...ageGroups,
    [ageKey]: (ageGroups[ageKey] || []).concat(p)
  }
}

// Combine the reducers
const seed = { idIndex: {}, idNameIndex: {}, ageGroups: {} };
const reducer = ({ idIndex, idNameIndex, ageGroups }, p) => ({
  idIndex: idIndexReducer(idIndex, p),
  idNameIndex: idNameIndexReducer(idNameIndex, p),
  ageGroups: ageGroupReducer(ageGroups, p)
})

const DATA = [
  {id: 1, name: 'Alan', date: '2021-01-01', age: 0},
  {id: 2, name: 'Ben', date: '1980-02-02', age: 41},
  {id: 3, name: 'Clara', date: '1959-03-03', age: 61},
]

// Loop once
console.log(
  JSON.stringify(DATA.reduce(reducer, seed), null, 2)
);

Subjective part: Whether it's worth it? I don't think so. I like simple code, and in my own experience going from 1 to 3 loops when working with limited data sets usually is unnoticeable.

So, if using Ramda, I'd stick to:

const { prop, indexBy, map, groupBy, pipe } = R;

const DATA = [
  {id: 1, name: 'Alan', date: '2021-01-01', age: 0},
  {id: 2, name: 'Ben', date: '1980-02-02', age: 41},
  {id: 3, name: 'Clara', date: '1959-03-03', age: 61},
];

const byId = indexBy(prop("id"), DATA);
const nameById = map(prop("name"), byId);
const ageGroups = groupBy(
  pipe(
    prop("age"), 
    age => age > 60 ? "senior" : age > 40 ? "medior" : "junior"
  ),
  DATA
);

console.log(JSON.stringify({ byId, nameById, ageGroups }, null, 2))
<script src="https://cdn.jsdelivr.net/npm/ramda@0.27.1/dist/ramda.min.js"></script>
user3297291
  • 22,592
  • 4
  • 29
  • 45
  • Thank you, sir, Really inspired me! But in this case, what would you do to obtain the 3 outcomes in the real world? What solution will you take? As you said it is not worth it, would you do the same as my ```imperative``` way, or do you have a better one to show me? – dummy Apr 07 '21 at 13:02
  • I included how I would do it using Ramda. It loops over `DATA` three times, but (at least to me) it is immediately clear what's going on. It's basically like your own attempt, but with a slightly more readable `groupBy` version of the age logic. I don't think it's worth it to make the age label function point free. Makes it harder for me to read. – user3297291 Apr 07 '21 at 13:09
  • Yes, exactly, that is what I want to ask cuz I think it would take at least 3 loops. Using Ramda PROS: code looks prettier, more readable, CONS: loops 3 times; Not using Ramda PROS: loops 1 time, CONS: looks less pretty; So the real problem I guess is can anyone use Ramda and loop 1 time? If anyone can, what would that be? If no one can, is it worth it? – dummy Apr 07 '21 at 13:28
  • I think someone more familiar with Ramda could definitely create an elegant and easy to read version of the composed reducer example I wrote in plain js. I guess that would be the _have your cake and eat it too_ option Maybe you can give it a try yourself, starting with [`reduceBy`](https://ramdajs.com/docs/#reduceBy). – user3297291 Apr 07 '21 at 14:48