Convert strConflictedYesReviewers data into a single object which is grouped by ownerid and make array of opportunityid

Question

I get a dynamic string(strConflictedYesReviewers) which contains multiple user recordswhere each record is separated by semicolon and then each record represents ownerid and opportunityid separated by asterisk.

I have done it but looking for a better approach or code review.

const strConflictedYesReviewers = "88639280*198719943;88642547*198721749;88627345*198721749;88664734*198721749;88686221*198721749;88676217*198721749;88664734*198721749;88686221*198721749;88676217*198721749;"
.split(";")
.map(item => item.split("*"))     
.filter(item => !!item[0])
.map(item => ({ownerid: item[0], opportunityid: item[1]}))
.reduce(function(acc, curr) {
  (acc[curr["ownerid"]] = acc[curr["ownerid"]] || []).push(curr["opportunityid"]);
  return acc;
}, {});
   

console.log(strConflictedYesReviewers);

Answer 1

You are iterating over the generated array needlessly when creating the intermediate objects since you throw away those objects in the reducer.

If the filtering is there just for the final ;-split, and you can assume you will always have both datum, you can drop the whole filtering by removing /;$/ from the string beforehand — saving another iteration. If you know for sure the string will always have the final ;, a slice would be efficient.

It’s often a good idea to separate the reducer and give it a descriptive name, for example:

const byOwner = (acc, [owner, opportunity]) =>
  ((acc[owner] ||= []).push(opportunity), acc)

(Please note the stylistic choices here, of which many would advice against. Such as the comma operator, lack of semicolons, braces, and explicit return. Perhaps even the somewhat recent logical OR assignment.)

so the usage becomes clean <iterable>.reduce(byOwner, {}) (although, there is codependence between what you are reducing into, and the inside of the reducer).

If the filtering really is for the possible empty ownerids, then each additional iteration naturally increases runtime. One concept which combines the ease of use of the small mapping and filtering functions while keeping down the iterations (without requiring a bloated reducer), is a transducer:

const compose = f => g => a => f(g(a))

const filtering = pred => reducer => (acc, x) =>
  pred(x) ? reducer(acc, x) : acc

const mapping = fun => reducer => (acc, x) =>
  reducer(acc, fun(x))

const xform = compose
  (filtering (([x]) => x))
  (mapping (x => x.split("*")))

so the usage becomes str.split(";").reduce(xform(byOwner), {}).

The benefit isn’t going to be apparent on small lists or simple transducers, though.