Add multiple data attributes to elements using jQuery

Question

I'm looking for some help on how I can optimize adding multiple data attribute tags to elements, and really, any feedback at all.

Background

Client uses a analytics tool through a tag management application (Ensighten) that picks-up data attributes when links are clicked. I'm adding attributes when the DOM is ready to provide them with more information about what people are clicking, where they are clicking, etc.

init.js

Here is an example of my init.js file (Ensighten wraps this in a IIFE):

// global namespace window.analytics = window.analytics || {}; window.analytics.heatmapping = window.analytics.heatmapping || {}; window.analytics.heatmapping.header = { logo: function () { var $this = jQuery(this), name = $this.closest('ul.navL2').prev().text(), type = $this.attr('alt'), title = $this.attr('title'); window.analytics.utilities.setDataAttributes($this, { 'region': 'header', 'name': name, 'type': type, 'title': title, 'index': '1' }); } // ... more below, }; // initializing jQuery('.top a').each(window.analytics.heatmapping.header.logo); 

utilities.js

I have another custom javascript tag that houses all of the utility functions that we can reuse. This is where the setDataAttributes function is kept. Here is the setDataAttributes function with its supporting functions.

 /** * Set data attributes on an element * @param {object} element A jQuery object, typically we'll pass jQuery(this). * @param {object} dataAttributes The data attributes we wish to set */ window.analytics.utilities.setDataAttributes = function (element, dataAttributes) { var util = window.analytics.utilities, dataAnalyticsTagAttributes, if (util.hasDataAnalyticsTag(element)) { dataAnalyticsTagAttributes = util.parseDataAnalyticsTag(element); // merge objects $.extend(dataAttributes, dataAnalyticsTagAttributes); } dataAttributes = util.prefixAndTrimProperties(dataAttributes); element.attr(dataAttributes); }; /** * Prefixes the incoming objects keys with 'data-' and trims objects values * @param {object} dataAttributes * @return {object} dataAttributeWithPrefix */ window.analytics.utilities.prefixAndTrimProperties = function (dataAttributes) { var util = window.analytics.utilities, dataAttributesWithPrefix = {}, dataKeyWithPrefix, dataKey, dataValue for (dataKey in dataAttributes) { if (dataAttributes.hasOwnProperty(dataKey)) { // prefix key with data- and trim value dataKeyWithPrefix = util.addPrefixToKey(dataKey) dataValue = jQuery.trim(dataAttributes[dataKey]); // returns new prefixed and clean property in dataAttributesWithPrefix object dataAttributesWithPrefix[wedcsKeyWithPrefix] = dataValue; } } return dataAttributesWithPrefix; }; /** * Determines if input element has the data-analytics tag attibute * @param {object} element jQuery(this) * @return {Boolean} */ window.analytics.utilities.hasDataAnalyticsTag = function(element) { return element.is('[data-analyticstag]'); }; /** * adds the 'data-' prefix to the input string * @param {string} key The objects key it currently iterating on. * @return {string} */ window.analytics.utilities.addPrefixToKey = function (key) { return 'data-' + key; } /** * Parses the data-analytics attribute on * @param {object} element A jQuery object, typically we'll pass jQuery(this). * @return {object} An object with the properties index, linktype and cmpgrp */ window.analytics.utilities.parseDataAnalyticsTag = function (element) { var dataAnalyticsAttributeArray = element.attr('data-analyticstag').split('_'); return { 'index': dataAnalyticsAttributeArray[4].match(/\d$/), 'type': dataAnalyticsAttributeArray.splice(0, 4).join(':'), 'region': dataAnalyticsAttributeArray[3] }; }; 

Let me explain what the setDataAttributes function does:

1. it takes two arguments: element and dataAttributes (an object)
2. it checks to see if the element has a dataAnalytics tag (some links have a data-analytics tag that we can get some values from)
3. if the element does have the data-analyticstag, then we parse it and return an object (see parseDataAnalyticsTag) and merge it with the original dataAttributes object.
4. Next, we take the dataAttributes object and pass it into another function prefixAndTrimProperties where we prefix each key with 'data-' and trim each value, this function returns an object.
5. we take the returned object and pass it into element.attr(dataAttributes) where it then sets the data attributes for that specific element.

Questions

I'm currently reading Clean Code by Robert C. Martin, and I'm attempting to apply some of his practices around naming and functions - haven't made it to the rest of the book yet.

1. How does my naming look? I'm a little lost on the prefixAndTrimProperties function. In his book he states that you only want the function to do one thing, and my function is doing two - at least.
2. Am I splitting up my functions in a way that are more testable? For example, is it really necessary to just have a function like hasDataAnalyticsTag return true or false? How granular should I be getting? Is it overkill?
3. Any other advice?

Answer 1

I'm a little lost on the prefixAndTrimProperties function. In [Martin's] book he states that you only want the function to do one thing, and my function is doing two - at least.

It's true that a function should ideally only "do one thing". But what constitutes "one thing" is somewhat debatable. For instance, if you instead call your function prepareProperties then its "one thing" is to, well, prepare a properties object. That's an operation that counts as one thing in your context. Yes, it entails both trimming values and prefixing keys, but that's an implementation detail.

Am I splitting up my functions in a way that are more testable? For example, is it really necessary to just have a function like hasDataAnalyticsTag return true or false? How granular should I be getting? Is it overkill?

Probably a little overkill, yes. But I'm more concerned about its use - or lack thereof. In parseDataAnalyticsTag you don't use it, meaning you may get an exception: element.attr('data-analyticstag').split('_') will fail if the attribute doesn't exist, since attr() will returned undefined, which you can't split.

In fact, I'd say it'd be easier to simply call parseDataAnalyticsTag and have it return null or an empty object if there's no attribute to parse. Right now, you've split it into checking and parsing, but - as far as I can tell - you only need to check if you want to parse. And if you want to parse, you need to check. So that's "one thing".

So, how granular should it be? Enough to keep the code DRY. If you find yourself repeating something, extract it into a function. Conversely, combine dependent/sequential steps into a function, and call that "one thing".

By the way, there's a hint that you may be too diligent in splitting things up. The comments for addPrefixToKey say

// @param {string} key The objects key it currently iterating on. 

Who said anything about an object? Or iteration? Or a key for that matter? The function just takes an argument - any argument, really - and prepends "data-" to it. That's it. Its name and comments indicate that it was intended for or extracted from a very specific context, but the function itself really doesn't care. But if its intended use-case is so specific, it probably shouldn't be a separate function at all.

As to the code itself:

You're exposing all your functions in the window.analytics.utilities object, though you seem to only use one: setDataAttributes. So that's your API; the rest is - viewed from the outside - implementation details.

I'm also not a big fan of the parseDataAnalyticsTag function. For one, its @return comment lies: The object does not contain index, linktype and cmpgrp properties - it contains index, type and region. Boo.

It's also fragile and fairly tricky to follow, despite its short length. As mentioned, you assume that the attribute exists when you call split, but after that, you also assume that there are at least 8 elements in the resulting array. And the use of splice instead of slice makes it hard to keep track of indices, and requires things to happen in the right order. I.e. the region value is actually index 7 - not 3 - in the original array, so it only works because you've used splice.

Lastly, setDataAttributes has side effects: You're modifying the dataAttributes object you're given. It's ok for the usage you've got right now, since you're not keeping a reference to the object on the caller's side, but it's icky nonetheless.

Suggestions:

I'd consider making this a jQuery plugin. You're depending on jQuery anyway.

Something like this, perhaps (note: incomplete implementation)

// get any existing attributes from the `data-analyticstag` attribute (if present) function analyticsTagAttributes(element) { // ... see current implementation, and all the stuff above ... } // Prefixes keys, and trims values function prepareAttributes(object) { var key, prepared = {}; for(key in object) { if(object.hasOwnProperty(key)) { prepared["data-" + key] = $.trim(object[key]); } } return prepared; } // extend jQuery $.fn.extend({ setAnalyticsAttributes: function (attributes) { return this.each(function () { var prepared = prepareAttributes(attributes), existing = analyticsTagAttributes(this) || {}, merged = $.extend(existing, prepared); $(this).attr(merged); }); } }); 

With that, you can simply call

$(elementOrSelector).setAnalyticsAttributes({ region: 'header', name: name, type: type, title: title, index: '1' }); 

Or, if you want to mimic jQuery, you make it a analytics function, so you can use it much like .attr():

$(elementOrSelector).analytics() // returns existing values $(elementOrSelector).analytics(obj) // sets values