2

I have an array of dailyEntries. A DailyEntry has an unique identifier id. I get an external dailyEntry and want to replace the dailyEntry inside the array with the external one.

I do this with:

this.dailyEntries = this.dailyEntries.map(function (dailyEntry) {
  if (dailyEntry.id === externalEntry.id) {
    dailyEntry = externalEntry;
  }
  return dailyEntry;
});

Is there a better way to do it? That seems like a bit too much code for a simple replacement.

Edit: I guess i can do the same thing in one line:

this.dailyEntries = this.dailyEntries.map(dailyEntry => dailyEntry.id === entry.id ? entry : dailyEntry);

But is there stil a better solution?

M.Dietz
  • 900
  • 10
  • 29
  • Do you know for sure that it exists in the array, or do you need to handle adding it if not (the code above isn't). – T.J. Crowder Nov 29 '19 at 15:11
  • _"That seems like a bit too much code for a simple replacement"_ - The "replacement" is only one line. The rest tries to find the element you want to replace. – Andreas Nov 29 '19 at 15:15
  • Possible duplicate of [Javascript ES6/ES5 find in array and change](https://stackoverflow.com/questions/35206125/javascript-es6-es5-find-in-array-and-change) – altocumulus Nov 29 '19 at 15:37

1 Answers1

3

But is there stil a better solution?

Not really. The code can be more concise (edit: you edited your question to say that as well):

this.dailyEntries = this.dailyEntries.map(dailyEntry => dailyEntry.id === externalEntry.id ? externalEntry : dailyEntry);

Or it's common to use e for the parameter in that kind of operation, since it's a very small function (e stands for "element"):

this.dailyEntries = this.dailyEntries.map(e => e.id === externalEntry.id ? externalEntry : e);

Another way is findIndex, but it doesn't seem as good as your map solution:

const index = this.dailyEntries.findIndex(({id}) => id === externalEntry.id);
this.dailyEntries[index] = externalEntry;

(That uses destructuring to pick out the id from the entry parameter to the map callback.)

The big difference between them is that the map solution creates a new array, but the findIndex solution updates the existing array (which has the advantage that it short-circuits when the entry is found; but the disadvantage that sometimes you want to treat objects [including arrays] as though they were immutable).

You could always give yourself a reusable function for this operation:

function replaceInArray(array, propName, newEntry) {
    return array.map(e => e[propName] === newEntry[propName] ? newEntry : e);
}

then

this.dailyEntries = replaceInArray(this.dailyEntries, "id", externalEntry);

All of the above assumes (like your current code) that the entry definitely exists in the array. If the entry may not be in the array, that makes map less attractive and suggests findIndex is the better way to go:

const index = this.dailyEntries.findIndex(({id}) => id === externalEntry.id);
if (index === -1) {
    this.dailyEntries.push(externalEntry);
} else {
    this.dailyEntries[index] = externalEntry;
}

Last but not least: If you're referring to the entry by its id in multiple places, an array may not be the right data structure (but it may well be, it depends on what else you're doing with it). For instance, you might keep these in a Map instead (or an object). Then replacing it (or adding it if not there) is really simple:

this.dailyEntries.set(externalEntry.id, externalEntry);

If you need an array of the entries (in the order in which they were added), you can get it from the iterable from the Map's values method: [...this.dailyEntries.values()] (or just use the iterable directly, if you don't need an array per se).

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875