Skip to content

Commit

Permalink
Use named arguments when creating tracker
Browse files Browse the repository at this point in the history
  • Loading branch information
fofr committed Feb 25, 2015
1 parent df477da commit ff058c2
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 7 deletions.
6 changes: 5 additions & 1 deletion app/assets/javascripts/analytics/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
var cookieDomain = (document.domain == 'www.gov.uk') ? '.www.gov.uk' : document.domain;

// Configure profiles, setup custom vars, track initial pageview
var analytics = new GOVUK.Tracker('UA-26179049-7', 'UA-26179049-1', cookieDomain);
var analytics = new GOVUK.Tracker({
universalId: 'UA-26179049-7',
classicId: 'UA-26179049-1',
cookieDomain: cookieDomain
});

// Make interface public for virtual pageviews and events
GOVUK.analytics = analytics;
Expand Down
6 changes: 3 additions & 3 deletions app/assets/javascripts/analytics/tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
"use strict";
window.GOVUK = window.GOVUK || {};

var Tracker = function(universalId, classicId, cookieDomain) {
var Tracker = function(args) {
var classicQueue,
setDimension = this.setDimension.bind(this);

classicQueue = getClassicAnalyticsQueue();
resetClassicAnalyticsQueue();

this.universal = new GOVUK.GoogleAnalyticsUniversalTracker(universalId, cookieDomain);
this.classic = new GOVUK.GoogleAnalyticsClassicTracker(classicId, cookieDomain);
this.universal = new GOVUK.GoogleAnalyticsUniversalTracker(args.universalId, args.cookieDomain);
this.classic = new GOVUK.GoogleAnalyticsClassicTracker(args.classicId, args.cookieDomain);

setPixelDensityDimension();
setHTTPStatusCodeDimension();
Expand Down
10 changes: 7 additions & 3 deletions spec/javascripts/analytics/tracker-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ describe("GOVUK.Tracker", function() {
window._gaq = [];
window.ga = function() {};
spyOn(window, 'ga');
tracker = new GOVUK.Tracker('universal-id', 'classic-id', '.www.gov.uk');
tracker = new GOVUK.Tracker({
universalId: 'universal-id',
classicId: 'classic-id',
cookieDomain: '.www.gov.uk'
});
});

describe('when created', function() {
Expand Down Expand Up @@ -41,7 +45,7 @@ describe("GOVUK.Tracker", function() {
window.ga.calls.reset();
window._gaq = [];
spyOn(GOVUK, 'cookie').and.returnValue("_setCustomVar,21,name,value,3");
tracker = new GOVUK.Tracker('universal-id', 'classic-id');
tracker = new GOVUK.Tracker({universalId: 'universal-id', classicId: 'classic-id'});
universalSetupArguments = window.ga.calls.allArgs();

expect(window._gaq[6]).toEqual(['_setCustomVar', 21, 'name', 'value', 3]);
Expand All @@ -56,7 +60,7 @@ describe("GOVUK.Tracker", function() {
['_setCustomVar', 21, 'name', 'value', 3],
['_setCustomVar', 10, 'name-2', 'value-2', 2]
];
tracker = new GOVUK.Tracker('universal-id', 'classic-id');
tracker = new GOVUK.Tracker({universalId: 'universal-id', classicId: 'classic-id'});
universalSetupArguments = window.ga.calls.allArgs();
});

Expand Down

4 comments on commit ff058c2

@bradwright
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍. I assume this breaks if we don't pass the values but we can sort that later.

@jabley
Copy link
Contributor

@jabley jabley commented on ff058c2 Feb 25, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style question.

I normally use args to mean an array, and opts / options to mean an object like that. Is there an established pattern in this codebase?

@edds
Copy link
Contributor

@edds edds commented on ff058c2 Feb 25, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the styleguide we use the word options when describing this pattern. But there is nothing explicitly written down about the naming of that variable.

@fofr
Copy link
Contributor Author

@fofr fofr commented on ff058c2 Feb 25, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose args over options because the arguments aren't yet optional. @bradleywright's assumption is correct.

Please sign in to comment.