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); } });
});
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);