Declaring JavaScript variable and then overwriting it multiple times

Question

I'm wondering if I am doing this in the most efficient way.

So I'm declaring my function el which is equal to a macro (Google Tag Manager), then I reassign or overwrite that previously-defined var which grabs the innerText/textContent of the element. It then converts the string to lowerCase and then capitalizes the first letter of the string, before finally returning the 'cleaned-up' element.

 function() {
 var el = {{element}}
 el = (el.innerText || el.textContent);
 el = el.toLowerCase();
 el = el.charAt(0).toUpperCase() + el.slice(1);
 return el;
 }

Answer 1

You can using chaining, move the toLowerCase() to simplify a little and return the last statement directly:

function() {
 var el = {{element}};
 el = (el.innerText || el.textContent);
 return el.charAt(0).toUpperCase() + el.slice(1).toLowerCase();
}

I'm personally not a fan of using a single variable for many purposes (I think it interferes with readability and maintainability) so I would probably do this:

function() {
 var el = {{element}};
 var text = (el.innerText || el.textContent);
 return text.charAt(0).toUpperCase() + text.slice(1).toLowerCase();
}

• jfriend00: Simplified a bit further by returning the last statement directly (one less line and assignment) and moved .toLowerCase() to operate on the final result.

Answer 2

I think you have a valid concern about the repeated reassignment.

Based on the Single-Responsibility Principle, I would split this function into two: one that interfaces with the DOM, and another that transforms a string.

function content(element) {
 var el = {{ element }};
 return el.innerText || el.textContent;
}
function titleCase(str) {
 return str.charAt(0).toUpperCase() + str.substr(1).toLowerCase();
}
// The original function in question
function() {
 return titleCase(content(element));
}

Decomposing the problem that way addresses the code smell at the root cause.

• konijn: Splitting functionality into 1 liners that you use only once usually fails code review. -1
• David Harkness: @konijn Big assumption with "you'll only use once". titleCase and content look like they could be reused a lot assuming this application has more than one DOM element. Besides, these methods are now testable and composable which can be a big win.
• David Harkness: Save possible upper-to-lower-to-upper conversion of the first character with str.substr(1).toLowerCase().
• 200_success: @DavidHarkness The order of operations will make a negligible difference in performance, but I think that the parallelism of putting toUpperCase()/toLowerCase() last is appealing. I've incorporated the change into Rev 2.
• Julix: content isn't a verb, functions do stuff - what does it do? getConetentFromElement or just getContent?