/**
* 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, typelinktype and regioncmpgrp
*/
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]
};
};
- How does my naming look? I'm a little lost on the
prefixAndTrimPropertiesfunction. In his book he states that you only want the function to do one thing, and my function is doing two - at least. - 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
hasDataAnalyticsTagreturntrueorfalse? How granular should I be getting? Is it overkill? - Any other advice?
EDIT: IN RESPONSE TO @Flambino ANSWER
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.
The "done one thing" is makes sense - subjective implementation detail.
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.
I'm using hasDataAnalyticsTag in the if statement in the setDataAttributes function. So the parse only runs if the element has a data-analyticstag, but I do like your idea about doing the checking and parsing in one function. Makes sense, just couldn't think of a way of how to do that. I see how to do that now thanks to your suggestions.
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.
addPrefixToKey - good point. I'll remove it and add the data string to the key itself e.g. ['data-' + key]. Didn't realize I could do a string concat within the [].
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.
Right, probably don't need to expose all of my functions to utilities. Good point.
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.
Yes, it does lie. It was a typo (updated now). Also, I've combined the hasDataAnalyticsTag into the parseDataAnalyticsTag per your previous comment. Also, I meant to use slice, not splice, so again, nice catch there.
Suggestions: I'd consider making this a jQuery plugin. You're depending on jQuery anyway.
I would love to be able to create a jQuery plugin; however, in a lot of the each functions, I'm having to do long if/elseif statemetnts, or some other logic to extract text from the page, so it would be as easy as just dropping in, say, $this.closest('ul.navL2').prev().text(), for the value of name.
So, for example, an example would be like - very ugly:
$(elementOrSelector).setAnalyticsAttributes({
region: 'header',
name: name,
type: type,
title: title,
index: function ($(this)) {
var region = jQuery.trim($this.closest('ul.navL2').prev().text());
if (region === 'purple' || region === 'green') {
index = '1';
} else if (region === 'red' || region === 'orange') {
index = '2';
} else if (region === 'yellow') {
index = '3';
} else if (region === 'blue') {
index = '4';
} else if (region === 'pink') {
index = '5';
} else if (region === 'white') {
index = '6';
}
return index;
}
});
Updated utilities.js:
window.analytics = window.analytics || {};
window.analytics.utilities = window.analytics.utilities || {};
// object to store private functions
var _ = {
/**
* Prefixes the incoming objects keys with 'data-' and trims incoming objects values
* @param {object} object
* @return {object} prepared
*/
prepareAttributes: function (object) {
var prepared = {},
key;
for (key in object) {
if (object.hasOwnProperty(key)) {
prepared['data-' + key] = jQuery.trim(object[key]);
}
}
return prepared;
},
/**
* Parses the data-analytics attribute
* @param {object} element A jQuery object, typically we'll pass jQuery(this).
* @return {object} An object with the properties region, type and index, or empty
*/
dataAnalyticsAttributes: function (element) {
var elementHasDataAnalyticsTag = element.is('[data-analyticstag]'),
parsed = {},
dataAnalyticsAttributes = [];
if (elementHasDataAnalyticsTag) {
dataAnalyticsAttributes = element.attr('data-analyticstag').split('_');
parsed.region = dataAnalyticsAttributes[4].match(/\d$/);
parsed.type = dataAnalyticsAttributes.slice(0, 3).join(':');
parsed.region = dataAnalyticsAttributes[3];
}
return parsed;
}
};
/**
* Set data attributes on an element
* @param {object} element A jQuery object, typically we'll pass jQuery(this).
* @param {object} attributes The data attributes we wish to set
*/
window.analytics.utilities.setWEDCSAttributes = function (element, attributes) {
var dataAttributes = _.dataAnalyticsAttributes(attributes),
merged = jQuery.extend(attributes, dataAttributes),
prepared = _.prepareAttributes(merged);
element.attr(prepared);
};
Remaining Questions
- In your code example where you getting the
analyticsTagAttributes(this) || {}, I thought it would be cleaner to still check for the data-analyticstag in the function, and if it exists, use it, if not then an empty{}is returned. Am I doing an unnecessary step? - Again, really like your idea of a plugin, but with the example I provided, it doesn't make for the cleanest of code, right?
