Merging values into an object

Question

I created an algorithm that helps me merging a payload into a given fixed object, in other words

this payload

const requestPayload = {
 "customer_email": "[email protected]",
 "customer_name": "unit test",
 "customer_firstname": "unit",
 "customer_lastname": "test",
 "customer_phone": "123456789",
 "customer_ts": "11.10.2020 12:37:39",
 "sc_device_type": "mobile",
 "sc_url": "http://www.domain.com"
}

with this setup/settings

const settings = {
 "n": 10002,
 "api_key": "pk_f434ea5e9c31906639b31076b24805be7a",
 "email": "customer_email",
 "name": "customer_name",
 "device": "sc_device_type",
 "url": "sc_url",
 "object1": {
 "device": "sc_device_type",
 },
 "object2": {
 "devices": {
 "device": "sc_device_type"
 }
 },
 "array1": [{
 "email": "customer_email",
 "name": "customer_name"
 }],
 "array2": [{
 "emails": [ "customer_email" ],
 "name": "customer_name"
 }],
 "array3": [4, "customer_email","customer_name","customer_ts"]
}

should output

{
 "n": 10002,
 "api_key": "pk_f434ea5e9c31906639b31076b24805be7a",
 "email": "[email protected]",
 "name": "unit test",
 "device": "mobile",
 "url": "http://www.domain.com",
 "object1": { "device": "mobile" },
 "object2": { "devices": { "device": "mobile" } },
 "array1": [{ "email": "[email protected]", "name": "unit test" }],
 "array2": [{ "emails": [ "[email protected]" ], "name": "unit test" }],
 "array3": [ 4, "[email protected]", "unit test", "11.10.2020 12:37:39" ]
}

---

Code works fine, but I wonder if anyone can help me with hints if I can make the code a bit better.

This is my 3rd refactored code, as I've started with a huge and terrible code, and only now I've added support to replace Arrays. As I'm new here, would love to take advantage of your expertise 😊

const templateParse = (entry) => {
 // TODO: add later, so we can pass a value like 'TML|DATE|yyyy-mm-dd' and will output '2020-11-29' (using today's date)
 return entry
}
const getTypeOfValue = v => Array.isArray(v) ? 'array' : typeof v
const replaceValue = (originalValue, dataToMerge, encodeOutput) => {
 if (typeof originalValue === 'string' && originalValue.indexOf('TMPL') === 0) {
 return templateParse(originalValue)
 } else {
 // check if property exist, use original value if not
 const mergedValue = Object.prototype.hasOwnProperty.call(dataToMerge, originalValue) ? dataToMerge[originalValue] : originalValue
 return encodeOutput ? encodeURI(mergedValue) : mergedValue
 }
}
const mergeArrayData = (originalValue, dataToMerge, encodeOutput) => {
 const arrayOutput = []
 const mergedArray = mergeCustomFields(originalValue, dataToMerge, encodeOutput)
 for (i in mergedArray) {
 arrayOutput.push(mergedArray[i])
 }
 return arrayOutput
}
const mergeCustomFields = (customFields, dataToMerge, encodeOutput = false) => {
 const mergeFields = {}
 for (key in customFields) {
 const v = customFields[key]
 const t = getTypeOfValue(v)
 if (t === 'array') {
 mergeFields[key] = mergeArrayData(v, dataToMerge, encodeOutput)
 } else if (t === 'object') {
 mergeFields[key] = mergeCustomFields(v, dataToMerge, encodeOutput)
 } else { // string or number
 mergeFields[key] = replaceValue(v, dataToMerge, encodeOutput)
 }
 }
 return mergeFields
}

and I'll call as mergeCustomFields(settings, requestPayload) having the option to pass true to encode values that might have unicode chars...

Answer 1

Naming Your variable names aren't quite as precise as they could be.

• Single-letter variables in particular should be avoided in most cases, except perhaps for the i of a for loop.

• mergeCustomFields performs the algorithm over an object. mergeArrayData performs the algorithm over an array. Maybe call them mergeArray and mergeObject.

• template (which you're already using in some places) is a great precise name for the data structure, much better than originalValue IMO.

Use startsWith to check if a string starts with another string - it's cleaner and easier to make sense of than an indexOf check against 0. (If you're worried about browser compatibility, use a polyfill, as always - keep the source code as reaadable as possible.)

Overly defensive code is verbose, weird, and usually an indicator of an issue earlier in the pipeline. Do you really need to do

const mergedValue = Object.prototype.hasOwnProperty.call(dataToMerge, originalValue) ? dataToMerge[originalValue] : originalValue

? If at all possible, consider changing it to something like:

const mergedValue = dataToMerge[originalValue] ?? originalValue

(Even if you aren't using hasOwnProperty anymore, if you have code that has a chance of creating objects with a hasOwnProperty key that does not refer to Object.prototype.hasOwnProperty, you should consider if there are ways to refactor it so that isn't an edge case that you have to keep in mind)

Extract the transformation of a single value into a standalone function - this will help separate concerns and will allow for a great improvement with .map, used below.

Always declare variables before using them - for (key in customFields) { creates a global key variable, or will throw an error in strict mode. (Another section with this same problem: for (i in mergedArray) {) Consider using a linter to automatically prompt you to fix these sorts of mistakes. (Use for (const key in customFields) { instead). But both of these sections have better methods available:

To create an array by transforming every element from another array, use .map instead of iterating over indicies and using .push:

const mergeArray = arr => arr.map(transformValue);

Use Object.entries to get the key and the value from an object at once, instead of iterating over the keys and separately extracting the values.

const mergeObject = (customFields) => {
 const mergeFields = {}
 for (const [key, value] of Object.entries(customFields)} {
 mergeFields[key] = transformValue(value);
 }
 return mergeFields
}

You could also consider using Object.fromEntries and mapping the original object's entries, which I think is even more appropriate when transforming all the values of one object (see below). But first, there's something else to consider that you might've noticed are missing from the above snippets. You're passing along dataToMerge and encodeOutput to every single function, but both of those variables are only ultimately used inside the replaceValue function. Consider either passing the replaceValue function instead, created with two constant parameters bound to it already, or declaring all the functions such that they have scope of a replaceValue function that has scope of the two needed values:

const requestPayload={customer_email:"[email protected]",customer_name:"unit test",customer_firstname:"unit",customer_lastname:"test",customer_phone:"123456789",customer_ts:"11.10.2020 12:37:39",sc_device_type:"mobile",sc_url:"http://www.domain.com"},settings={n:10002,api_key:"pk_f434ea5e9c31906639b31076b24805be7a",email:"customer_email",name:"customer_name",device:"sc_device_type",url:"sc_url",object1:{device:"sc_device_type"},object2:{devices:{device:"sc_device_type"}},array1:[{email:"customer_email",name:"customer_name"}],array2:[{emails:["customer_email"],name:"customer_name"}],array3:[4,"customer_email","customer_name","customer_ts"]};
const templateParse = (entry) => {
 // TODO: add later, so we can pass a value like 'TML|DATE|yyyy-mm-dd' and will output '2020-11-29' (using today's date)
 return entry
}
const getTypeOfValue = v => Array.isArray(v) ? 'array' : typeof v
const mergePayload = (template, replacements, encodeOutput = false) => {
 const replaceValue = (originalValue) => {
 if (typeof originalValue === 'string' && originalValue.startsWith('TMPL')) {
 return templateParse(originalValue)
 } else {
 // check if property exist, use original value if not
 const mergedValue = replacements[originalValue] ?? originalValue
 return encodeOutput ? encodeURI(mergedValue) : mergedValue
 }
 };
 const mergeArray = arr => arr.map(mergeValue);
 const mergeValue = (value) => {
 const t = getTypeOfValue(value)
 return t === 'array'
 ? mergeArray(value)
 : t === 'object'
 ? mergeObject(value)
 : replaceValue(value);
 };
 const mergeObject = customFields => Object.fromEntries(
 Object.entries(customFields).map(
 ([key, value]) => [key, mergeValue(value)]
 )
 );
 return mergeObject(template);
};
console.log(mergePayload(settings, requestPayload));

That's the version I'd prefer - were it not for the JSON approach mentioned below.

There's one potential pitfall which may or may not be something to worry about: typeof null will give object. If null is a possible value in the template, change the getTypeOfValue function to account for it. (you could return 'null' or whatever you wanted, anything but 'object', so that mergeObject later doesn't throw)

But this whole thing could be made much simpler by exploiting the naturally recursive nature of JSON.stringify, and using a reviver function:

const requestPayload={customer_email:"[email protected]",customer_name:"unit test",customer_firstname:"unit",customer_lastname:"test",customer_phone:"123456789",customer_ts:"11.10.2020 12:37:39",sc_device_type:"mobile",sc_url:"http://www.domain.com"},settings={n:10002,api_key:"pk_f434ea5e9c31906639b31076b24805be7a",email:"customer_email",name:"customer_name",device:"sc_device_type",url:"sc_url",object1:{device:"sc_device_type"},object2:{devices:{device:"sc_device_type"}},array1:[{email:"customer_email",name:"customer_name"}],array2:[{emails:["customer_email"],name:"customer_name"}],array3:[4,"customer_email","customer_name","customer_ts"]};
const mergePayload = (template, replacements) => JSON.parse(
 JSON.stringify(template),
 (key, value) => replacements[value] ?? value
);
 
console.log(mergePayload(settings, requestPayload));