1
\$\begingroup\$

I have a function that creates a css transform property string based on some global variables. This string can be copied later by a user, so I would prefer to keep the output short.

This is my code:

var createTransformation = () => {
 var final_transformation = "";
 if (x)
 final_transformation += "rotateX(" + x + "deg)";
 if (y)
 final_transformation += " rotateY(" + y + "deg)";
 if (z)
 final_transformation += " rotateZ(" + z + "deg)";
 if (scalex !== 1 || scaley !== 1)
 final_transformation += " scale(" + scalex + "," + scaley + ")";
 if (translatex !== 0 || translatey !== 0 || translatez !== 0) {
 if (translatex !== 0 && translatey == 0 && translatez == 0)
 final_transformation += " translateX(" + translatex + "px)";
 else if (translatey !== 0 && translatex == 0 && translatez == 0)
 final_transformation += " translateY(" + translatey + "px)";
 else if (translatez !== 0 && translatex == 0 && translatey == 0)
 final_transformation += " translateZ(" + translatez + "px)";
 else if (translatex == 0)
 final_transformation += " translateY(" + translatey + "px) translateZ(" + translatez + "px)";
 else if (translatez == 0)
 final_transformation += " translateX(" + translatex + "px) translateY(" + translatey + "px)";
 else if (translatey == 0)
 final_transformation += " translateX(" + translatex + "px) translateZ(" + translatez + "px)";
 else
 final_transformation += " translateX(" + translatex + "px) translateY(" + translatey + "px) translateZ(" + translatez + "px)";
 }
 //if scalex != 1 || scaley != 1 
 //If the skew's sliders have been changed e.g, moved from 0 to 1 or from 0 to 100 add skew property
 if (skewX || skewY)
 final_transformation += " skew(" + skewX + "deg," + skewY + "deg)";
 return final_transformation;
 };

As you can see there are a lot of if statements. I was wondering, is there anything I can do to improve this code?

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jan 20, 2017 at 16:58
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

I would strongly consider storing the various variables that define the transformation in a structured object like this:

let transformConfig = {
 rotate: {
 x: ...,
 y: ...,
 z: ...
 },
 scale: {
 x: ...,
 y: ...
 },
 translate: {
 x: ...,
 y: ...,
 z: ...
 },
 skew: { 
 x: ...,
 y: ...
 }
};

That gives you the ability to easily iterate over this object to create transformation strings. An example of this iteration might be:

function createTransformString(transformConfig) {
 // a variable to store array of individual transform strings
 var transformStrings = [];
 // store functions that build strings for each transform type
 // not shown here but you may want to consider ECMA6 templating
 var buildString = {
 rotate: function(rotate) {
 for (dim in rotate) {
 transformStrings.push( 'rotate' + dim + '(' + rotate[dim] + 'deg)' );
 }
 },
 scale: function(scale) {
 scale.x = scale.x || 1;
 scale.y = scale.y || 1;
 transformStrings.push( 'scale(' + scale.x + ',' + scale.y + ')' ); 
 },
 translate: function(translate) {
 for (dim in translate) {
 transformStrings.push( 'translate' + dim + '(' + translate[dim] + 'px)' );
 }
 },
 skew: function(skew) {
 skew.x = skew.x || 0;
 skew.y = skew.y || 0;
 transformStrings.push( 'skew(' + skew.x + 'deg,' + skew.y + 'deg)' );
 }
 }; 
 // iterate the config object to generate strings
 for (transformAction in transformConfig) {
 let transform = transformConfig[transformAction];
 buildString[transformAction](transform);
 }
 return transformStrings.join(' ');
};

Usage:

let transformConfig = { ... };
let transformString = createTransformString(transformConfig);

This approach would limit the repeat code you have for each transformation string and would also take care of your if concern in that only those transformation types specified in the config would be executed.

This also more directly injects the transformation configuration information into the function that generates the strings, meaning you no longer have to rely on variables that may or may not be present in inherited scope. This means you could now have your createTransformString() function declared in a wholly separate include file from the code where the actual transformation(s) values are generated, promoting code re-use.

Since you are using ECMA6, I would also consider building this as a class (or classes).

Finally, from a stylistic standpoint, I would recommend sticking with camelCase instead of snake_case, as this is pretty much the de facto standard for javascript. At a minimum, don't intermingle the two. Additionally, I would suggest watching your line length. You should strive to keep lines of code under ~80 characters per line to improve code readability.

answered Jan 20, 2017 at 18:12
\$\endgroup\$
3
\$\begingroup\$

First I would suggest using template literals. This avoids you from splitting the string, and potentially breaking it.

final_transformation += `rotateX(${x}deg)`;

Next, instead of concatenating final_transformation manually, consider storing those values in an array first, then collapsing them to a string with array.join at the end.

const transforms = [];
if(...) transforms.push(`rotateX(${x}deg)`);
...
const finalTransformations = transforms.join(' ');

It appears that your if statements are simply checking for value presence. A way to avoid this is to simply assign them values. That way, they always have values. It's the same concept as data types. If a certain variable is of type Number, it should exist and it should be a Number.

Also, if x always had a number, you can always add rotateX. It doesn't matter if the value is 0, since rotateX(0deg) will do nothing anyways.

answered Jan 20, 2017 at 18:11
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.