Javascript Refactoring - Eliminating duplication from code which depends on an object of lists with-CodePudding

Introduction

Imagine 10 different functions:

const a = async (uid, data) => { ... };
const b = async (uid, data) => { ... };
const c = async (uid, data) => { ... };
const d = async (uid, data) => { ... };
const e = async (uid, data) => { ... };
const f = async (uid, data) => { ... };
const g = async (uid, data) => { ... };
const h = async (uid, data) => { ... };
const i = async (uid, data) => { ... };
const j = async (uid, data) => { ... };

And 4 different keys:

"key1", "key2", "key3", "key4"

I have created the following object:

const OBJECT = { // constant, will always look like this
  key1: [a, b, c, d, e, f, h, i, j],
  key2: [a, b, c, d, f, i, j],
  key3: [a, b, c, d, e, f, g, h, i, j],
  key4: [a, b, c, d, e, f, h, i, j],
}

And I am about to use it as follows:

const uid = uuid.v4();

const dynamicData = { // can contains any key (key1, key2, key3, key4)
  key1,
  key2,
};

let selectedMethods = [];

Object.keys(dynamicData).forEach((key) => {
  if (key in OBJECT) {
    selectedMethods.push(...OBJECT[key]);
  }
});

// Uniquefy the list using lodash
selectedMethods = _.uniq(selectedMethods);

const promises = selectedMethods.map(
  (method) => method(uid, dynamicData)
);

await Promise.all(promises);

Problem

This code is not really clean... as you can see, I am repeating lot of stuff when initializing the object of arrays (seems like a raw implementation).

How can I change the way I am initializing it? Also, as you can see, there is a pattern in the different methods params (they all accept the same stuff). So, if you think it is possible to make this code less spaghetti, how can I do it?

CodePudding user response：

You can extract repeated parts out into a helper variable and spread it into the respective array literal:

const abcd = [a, b, c, d];
const ef = [e, f];
const ij = [i, j];
const OBJECT = {
  key1: [...abcd, ...ef, h, ...ij],
  key2: [...abcd, f, ...ij],
  key3: [...abcd, ...ef, g, h, ...ij],
  key4: [...abcd, ...ef, h, ...ij],
}

You should however only do this where it does make sense semantically. If ef has no further meaning in your business domain, it's likely to be coincidental duplication, and actually more work if the usage sites are likely to change independently. Also unnecessary abstraction makes code harder to understand.

Another approach that might be viable is to put the common prefix and suffix into a helper function:

const value = middle => [a, b, c, d, ...middle, i, j];
const OBJECT = {
  key1: value([e, f, h]),
  key2: value([f]),
  key3: value([e, f, g, h]),
  key4: value([e, f, h]),
}

You might as well remove them from the OBJECT altogether and just always add them to the selectedMethods list.