Also, there are a number of principles explained in a book written by Robert C. Martin (aka Uncle Bob) titled "Clean Code". There are many ideas in this book that will challenge some of the ways you think about code and will ultimately make it better in many ways.
One principle that is core to clean code is the Single Responsibility Principle (SRP). It's formal definition is that a class/module or method/function should have only one reason to change. However, when I first heard this definition, it didn't quite register with me. Another way I like to phrase it is a class/module or method/function should do only one thing. SRP results in small methods/functions, maybe less than 5 lines of code as well as classes/modules that are small consisting of maybe less than 5 methods/functions. This also leads to names that are a bit longer and more descriptive.
Here's a breakdown of what's going on in the getOpacity function:
- Obtains the filter property of the element style (elem.style.filter).
- If the filter property does not exist (is falsy), the opacity property of the element style (elem.style.opacity) is returned.
- If the filter property does exist, the filter is checked to see if it contains the text "opacity=".
- If the filter property doesn't contain "opacity=", it returns a zero-length string ("").
- If the filter property does contain "opacity=", it uses a regular expression to parse the opacity value, divides it by 100 and returns this value.
Let's take a closer look at the regular expression:
The two forward slashes indicate where the regular expression begins and ends. The string "opacity=" is just a string literal. The parenthesis indicate a capturing group which is a value that the regular expression parser will make available to us. Here's the capturing group:
The brackets indicate a character class. In a character class, you can specify multiple characters that should be used when matching a string value. In this case, the character class contains ^) meaning not a closing parenthesis.
The plus sign after the character class indicates we must have one or more matches resulting from it.
The match function returns an array containing the results of the regular expression matching. The first element contains the full string value that was matched. The following elements in the array contain the result of any capturing groups defined in the regular expression. In this case, we've defined one capturing group which would contain the opacity value defined for the HTML element.
The main issue with the getOpacity function is that the ternary expression was a bit overdone making it fairly hard to read. Additionally, one line of code is creating a regular expression, performing a regex match on a string, dereferencing the returned array from the regex match to obtain the opacity value, parsing it as a float value, dividing it by 100 and finally casting the result to a string (+ "").
Here's a refactored version of this code that adheres to the Single Responsibility Principle more closely:
Let's take a closer look at the refactored version of the code. First we define a few new functions as follows:
isElement: Returns true or false indicating whether the parameter passed is a DOM element or not.
isOpacityDefined: Returns true or false indicating whether the element style has a property containing the element opacity. Newer browsers will have this property available.
getOpacityFromStyle: Returns the opacity property from the element style.
Next we define the getOpacity function which contains the following functions:
areParametersValid: Returns true or false indicating whether the parameters provided to getOpacity are valid or not.
getRegex: Returns a regular expression used to parse the opacity value from an element's style attribute.
getFirstCapturingGroup: Returns the first capturing group found by a regular expression match.
castValue: Returns the value provided converted to a number.
shiftValue: Returns the value provided divided by 100.
Here's the logic contained within the getOpacity function:
- It determines whether the parameters passed are valid or not.
- If the parameters are not valid, it throws an error.
- It checks to see if the element's style has the opacity setting available.
- If the opacity setting is available, it returns this value.
- Gets the regular expression object needed to parse the opacity value.
- Obtains the style attribute from the element.
- Applies the regular expression to the style attribute resulting in matches.
- Retrieves the first capturing group from these matches.
- Casts the opacity value to a number.
- Shifts the decimal place in the opacity value two places to the left.
- Returns the resulting opacity value.
Here are my observations resulting from the code refactoring:
- The amount of code increased in size from 17 lines of code to 71 (an increase of 54 lines of code).
- The code much more clearly demonstrates what it's doing. It's much easier to read and reason about.
- Each function is focused and handles one responsibility.
- One could say the getOpacity function is doing multiple things. However, it's responsibility is to coordinate the various function calls and return the result.
- Three functions have been factored out of getOpacity: isElement, isOpacityDefined and getOpacityFromStyle. At some point, these functions could be moved into a HTML helper module making them available to other parts of the system.
- Now that we have several functions, each of them could be unit tested independently.
The final point I'd like to make comes directly from the book "The Art of Unix Programming" by Eric Steven Raymond:
"There are two ways of constructing a software design. One is to make it so simple that there are obviously no deficiencies; the other is to make it so complicated that there are no obvious deficiencies. The first method is far more difficult."