Last week i reviewed a small new feature on some old code. Without getting too much into details or ranting if it makes sense or not from a UX perspective, the request was along these lines:
When we add / create an author in the UI, if the author has published more than one book, in more than one language, we should expand the Author object and insert multiple rows in the DB. Basically one row for each Book/Language pair.
Imagine this author:
const author = {
name: "John",
surname: "Doe",
books: ["A novel", "Romance of your life", "Midnight Krimi"],
languages: ["IT","DE","EN"]
}
Since we have 3 books and 3 languages, we should duplicate the author 9 times ( where the book and language consist in only one item instead of an array.
The code to be reviewed looked something like this:
const cloneObject = (obj) => {
return JSON.parse(JSON.stringify(obj));
};
const cloneObjects = (entries, from, to) => {
const objects = [];
entries.forEach((obj) => {
if (obj.hasOwnProperty(from)) {
let valuesToSplit = obj[from];
if (typeof valuesToSplit === "string") {
valuesToSplit = valuesToSplit.split(",");
}
valuesToSplit.forEach((value) => {
const clonedObject = cloneObject(obj);
delete clonedObject[from];
if (typeof value === "string") {
clonedObject[to] = value;
}
if (typeof value === "object") {
clonedObject[to] = value[to];
}
objects.push(clonedObject);
});
} else {
objects.push(obj);
}
});
return objects;
};
const expandRequest = (request) => {
let entries = [request];
entries = cloneObjects(entries, "books", "book");
entries = cloneObjects(entries, "languages", "language");
return entries;
};
The good part of this code is that is designed to be generic enough so that the cloneObjects function can be iteratively invoked on different properties and that it takes into account a deep copy of the object to be cloned.
On the other hand, being generic was not in the requisites - the use case at hand was very specific to those two properties due to very old DB and Client implementations.
Even the deep clone was not necessary ( again, the objects, in this case, have always been flat and there is no point in using such an expensive and obscure operation like JSON.parse(JSON.stringify(obj)).
Other criticism to this implementation was that it was not functional - entries where constantly mutated and not immediately clear.
So let's see how this code could be refactored.
First of all, if current implementation makes it possible, before touching any code that works - no matter how ugly, unperformant, cumbersome it might be - we should have unit tests, so that we are 100% sure our refactoring does not break the expected behaviour.
import test from "ava"
test('Author is expanded into multiple objects (num of books x languages) when it has more than one book and more language/trnaslations', t => {
const author = {
name: "John",
surname: "Doe",
books: ["A novel", "Romance of your life"],
languages: ["IT","DE"]
}
const expected = [
{
name: "John",
surname: "Doe",
book: "A novel",
language: "IT"
},
{
name: "John",
surname: "Doe",
book: "A novel",
language: "DE"
},
{
name: "John",
surname: "Doe",
book: "Romance of your life",
language: "IT"
},
{
name: "John",
surname: "Doe",
book: "Romance of your life",
language: "DE"
}
]
const expanded = expandRequest(author)
t.is(expanded.length, author.books.length * author.languages.length)
t.deepEqual(expanded, expected)
})
Now we can proceed with the refactoring:
since we know that we can live with a shallow copy - object is flat anyway
we can change
JSON.parse(JSON.stringify(obj)
using the spread operator
const clone = {...obj}
then we can extract the arrays that we want to use as "multiplier" using destructuring:
const {books, languages} = obj;
and we write a method that iterate through the first array and map it to a new cloned object filled with a new property
const expandedWithBooks = books.map(b=> ({...clone, book:b}) )
then we use reduce to iterate over all the authors with a book, and we apply a similar function to clone each of them adding the language.
languages.reduce((acc, curr)=> {
const addLang = expandedWithBooks.map(o => ({ ...o, language:curr }))
return [...acc , ...addLang]
}
,[])
Notice the spread operator way concatenating two arrays:
[...array , ...anotherArray] is equivalent to array.concat(anotherArray) since both ways return a new Array.
Final method looks like this:
const expand = (obj) => {
const {books, languages} = obj;
const clone = {...obj}
delete clone["books"];
delete clone["languages"];
const expandedWithBooks = books.map(b=> ({...clone, book:b}) )
return languages.reduce((acc, curr)=> {
const addLang = expandedWithBooks.map(o => ({ ...o, language:curr }))
return [...acc , ...addLang]
}
,[])
}
I love ES6 features.
Top comments (0)