I'm trying to refactor the following node.js code.
Each case generates a thumbnail, chaining a different set of GraphicMagic transformation to an image.
switch(style.name) {
case 'original':
gm(response.Body)
.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option)
.toBuffer(function(err, buffer) { if (err) { next(err); } else { next(null, buffer); } });
break;
case 'large':
gm(response.Body)
.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option)
.quality(style.quality)
.strip().interlace('Plane')
.toBuffer(function(err, buffer) { if (err) { next(err); } else { next(null, buffer); } });
break;
case 'medium':
gm(response.Body)
.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option)
.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset)
.repage('+')
.strip().interlace('Plane')
.toBuffer(function(err, buffer) { if (err) { next(err); } else { next(null, buffer); } });
break;
case 'small':
gm(response.Body)
.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option)
.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset).repage('+')
.quality(style.quality)
.strip().interlace('Plane')
.toBuffer(function(err, buffer) { if (err) { next(err); } else { next(null, buffer); } });
break;
}
However, all cases share a number of transformations at the beginning and the end of the chaining, so there's room for refactoring. I tried refactoring with the following approach, but the code seems incorrect:
gm(response.Body)
.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option, function(err, response) {
if (style.name === 'original'){
return response;
} else if (style.name === 'large'){
return response.quality(style.quality)
} else if (style.name === 'medium'){
return response.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset).repage('+')
} else if (style.name === 'small'){
return response.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset).repage('+').quality(style.quality)
}
}).(function(response) {
return (stryle.name !== 'original') ? response.strip().interlace('Plane') : return response;
}).(function(argument) {
return response.toBuffer(function(err, buffer) { if (err) { next(err); } else { next(null, buffer); } });
});
Function chaining is a pattern in JavaScript where multiple functions are called on the same object consecutively. Using the same object reference, multiple functions can be invoked. It increases the readability of the code and means less redundancy.
Optional chaining is a safe and concise way to perform access checks for nested object properties. The optional chaining operator ?. takes the reference to its left and checks if it is undefined or null. If the reference is either of these nullish values, the checks will stop and return undefined.
To enable optional chaining, you need to install a package. At the time of writing, optional chaining is not natively supported in Javascript, it is a new feature introduced in ES2020. Until it is fully adopted we can get all the optional goodness by installing a package!
The optional chaining ?. is a safe way to access nested object properties, even if an intermediate property doesn't exist.
I wouldn't go for a switch
at all.
There is no reason to use chaining at all here. Just do
// if (!/^(original|large|medium|small)$/.test(style.name)) throw new Error(…);
var x = gm(response.Body)
.setFormat('jpg')
.autoOrient()
.resize(style.w, style.h, style.option);
if (style.name == "medium" || style.name == "small")
x = x.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset)
.repage('+');
if (style.name == "large" || style.name == "small")
x = x.quality(style.quality);
if (style.name == "large" || style.name == "medium" || style.name == "small")
// possibly better than if (style.name != "original")
x = x.strip()
.interlace('Plane');
x.toBuffer(next);
But if you're having a large set of options so that it gets unreadable, better factor out each transformation in a function:
function resizedJpg(x) {
return x.setFormat('jpg').autoOrient().resize(style.w, style.h, style.option);
}
function cropped(x) {
return x.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset).repage('+');
}
function withQuality(x) {
return x.quality(style.quality);
}
function stripped(x) {
return x.strip().interlace('Plane');
}
And then apply them separately:
({
original: [resizedJpg],
large: [resizedJpg, withQuality, stripped],
medium: [resizedJpg, cropped, stripped],
small: [resizedJpg, cropped, withQuality, stripped]
}[style.name]).reduce(function(x, trans) {
return trans(x);
}, gm(response.Body)).toBuffer(next);
I wrote small js helper for such usecases: https://github.com/sms-system/conditional-chain
With the help of this tool, you can chain methods optional, based on condition. Code will be more controlled and readable.
cond(gm(response.Body)
.setFormat('jpg')
.autoOrient()
.resize(style.w, style.h, style.option))
.if(style.name == 'medium' || style.name == 'small', gm => gm
.crop(style.w, style.h, style.crop.x_offset, style.crop.y_offset)
.repage('+'))
.if(style.name == 'large' || style.name == 'small', gm => gm
.quality(style.quality))
.if(style.name != 'original', gm => gm
.strip()
.interlace('Plane'))
.end().toBuffer(next)
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With