Lessons learnt today: Double quotes, redirection and rubbish routers

I haven’t posted here in a long while, and no doubt that will continue unfortunately. However, tonight I’ve learnt (or in some cases re-learnt) a few, albeit simple, lessons and it felt sensible to note it down so I can remember in the future.

Double Quotes vs. Single Quotes in PHP

This was a massive rookie error and a sign that I haven’t worked in PHP much over the past year.

While tightening my database security, I ended up patching up some dodgy db related code in PHP in a particularly old project. I spent nearly half an hour trying to work out why my passworded database user was being denied access to the database.

After a bit of debugging, I noticed the password was being cut off in the middle, and after further debugging and tracing the string back, I noticed that my randomly generated, double quoted password string happened to have a ‘$’ character in it.

PHP (among other languages) tries to resolve variables within double quoted strings, meaning “abc123$efg456” is resolved to “abc123”, if the variable $efg456 doesn’t exist in your script. The solution was to simply exchange the double quotes for single quotes.

Lesson: If you’re working in a language which treats double and single quoted strings differently, check you’re using the right ones!

.htaccess redirection

.htaccess always ends up leeching away my time. This time I was trying to set up some redirects to treat a sub-directory as the root directoy, but only if the file or directory didn’t exist in the root directory and did exist in the sub-directory.

This is simple enough if you know what the .htaccess variables mean, but in using examples and making assumptions I tripped myself up. So here’s the bit I learnt:

%{REQUEST_FILENAME} – This isn’t just the filename that was requested, but the absolute path from the root of the server.
%{REQUEST_URI} – This is the filename on its own.
%{DOCUMENT_ROOT} – This is usually the path up to the root directory of your site (though I’m quite sure this is not always the case).

So given the path “/a/file/path/to/a/website/index.html”:

%{REQUEST_FILENAME} = /a/file/path/to/a/website/index.html
%{REQUEST_URI} = index.html
%{DOCUMENT_ROOT} = /a/file/path/to/a/website

Simple when you know, but confusing otherwise! In any case, here’s the resulting rule I cobbled together:

RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{DOCUMENT_ROOT}/other%{REQUEST_URI} -f
RewriteRule ^(.*)$ /other/$1 [L,QSA]

That won’t suffice if you need directories to work as expected, and it will only apply to files, but it’s as much as I need for now.

Lesson: Don’t assume things, especially when dealing with something as powerful as .htaccess files. The more you know and use it, the less of a pain it will be.

NAT Loopback and remote IPs not working locally

Having acquired a new domain name today, I decided to put it to work as a domain for my home server (with help from no-ip). Having set it all up, I came across a peculiar scenario where I was able to access the machine remotely with the domain (the outward facing IP), I was able to access the machine locally with the local IP address, but I was unable to access the machine locally with the public IP or domain name.

In a few minutes I realised that this was not so peculiar at all. The Network Address Translation (NAT) rules decide where inbound requests should go when it hits the router, I have my router set up to allow certain connections to forward through to my server. However, these rules don’t apply to requests which pass through the router on the way out. I’d only be guessing, but I’d imagine this is because responses to requests across the Internet would otherwise have these rules applied to them as well, completely breaking the network.

To solve this issue, NAT loopback, a feature or further rule which resolves requests to the router’s public IP from inside the network correctly, is available in many routers. It is commonly turned off due to security concerns, or simply may not be available in some routers.

Unfortunately, my Huawei HG533 router falls into the latter group, with no obvious plans of an upgrade which would fix this.

Lesson: If you want to use one address to access a machine locally and remotely, ensure NAT Loopback is set up.

All simple stuff, but it’s been interesting learning about it all. Hopefully I can continue documenting the small things like this over the next year, my final year of university should be a steep learning curve!

Advertisements

Refactoring Code in to an Object-Oriented Paradigm #6: Compatibility

This is article number 6 of a series of article, which simply documents my refactoring of some code I originally wrote (pretty poorly) last year. To get the gist of what’s going on. Check the original post – Refactoring Code in to an Object-Oriented Paradigm.

Making Things Work for Everyone

We’re nearly finished with our refactor, we’ve added some great functionality, made things a lot cleaner and more efficient…that is for the browsers that can run our code. The problem is that, while cross-browser compatibility has got a lot easier, it’s still an issue. It’s only made worse by the legacy versions of IE which are still clinging on, and until the majority of IE users are on IE10, we’re still going to see some large differences or deficiencies for those users.

Because our code is quite simple, there aren’t a huge number of compatibility issues, but there are a few. These issues are also quite common when using Javascript. So we’ll sort them out, explaining whats and whys of each solution.

Event Handlers

I mentioned in the last article how I’d “continue side-stepping the compatibility issues which exist” regarding native event handling. The issue with native event handling is that there are two flavours that are definitely required and another that should be used to be safe. That doesn’t make for nice, quick code. But I wrote the code in the last article with extensibility in mind, so we can simply add compatibility for other browsers without affecting the rest of the code. Here’s our original code…

/** Set a native event listener (eventually regardless of the browser you're using).
*
*   @param eventElm The element to which the element to listen for, will involve.
*   @param eventType The type of event to listen for.
*   @param callback The function to call when the event happens.
*   @return True if listener was set successfully, false otherwise.
*/
AutoScrobbler.prototype.setNativeListener(eventElm, eventType, callback) {

eventElm.addEventListener(eventType, callback, false);

}

Conveniently, the MDN page for this has a bare-minimum shim documented, and details older methods of event handling as well. So we’ll use this as a guide on how to add compatibility to this function.

/** Remove a native event listener regardless of the browser you're using.
*
*   @param eventElm The element that had been listening.
*   @param eventType The type of event that was being listened for.
*   @param callback The function that was set to trigger.
*   @return Undefined if successful, otherwise an exception will be thrown.
*/
AutoScrobbler.prototype.setNativeListener(eventElm, eventType, callback) {

if (eventElm.addEventListener)

eventElm.addEventListener(eventType, callback, false);

else if (eventElm.attachEvent)

eventElm.attachEvent('on' + eventType, callback);

else {

eventElm['on' + eventType] = callback;
if (typeof eventElm['on' + eventType] == "undefined")

throw "EventHandler Error: The event could not be set";

}

}

This version includes far more browser support, and goes a step further in that it will even support the very old version of event handlers. However, it does currently assume that every single recognised event by addEventListener is simply an unprefixed version of the events recognised by both attachEvent and the older element.event style handler. This is certainly not true in some cases, for instance the “textInput” event isn’t at all support by attachEvent, the closest you can get to this event is “onpropertychange”. If we wanted this level of support, we would need to have an exceptions list, which would change these values based on what event handler was to be used. However, it works for simple ‘click’ events and ‘mouseover’ events. We can also add a complementing removeNativeListener function too.

/** Set a native event listener regardless of the browser you're using.
*
*   @param eventElm The element to which the element to listen for, will involve.
*   @param eventType The type of event to listen for.
*   @param callback The function to call when the event happens.
*   @return Undefined if successful, otherwise an exception will be thrown.
*/
AutoScrobbler.prototype.removeNativeListener(eventElm, eventType, callback) {

if (eventElm.removeEventListener)

eventElm.removeEventListener(eventType, callback, false);

else if (eventElm.detachEvent)

eventElm.detachEvent('on' + eventType, callback);

else

eventElm['on' + eventType] = undefined;
if (typeof eventElm['on' + eventType] != "undefined")

throw "EventHandler Error: The event could not be removed";

}

This should now provide us with the support we need for any event handling in the code.

Custom Event Handling

I originally implemented my own method of custom event handling in my article on making code extensible. I chose to do this due to the plethora of compatibility issues which come with custom event handling using native provisions. The most support for the new method was only fully implemented across browsers when IE9 came along. If we were to use it, it would mean that the code would only be available for use in “modern” browsers (so not IE8 or below). For the basic uses we want it for (namely making our code more extensible and future-proof), we don’t want to lose all that support.

There are libraries and shims out there which will make all event handling a one liner. Large libraries such as jQuery and MooTools were practically made for solving cross-browser issues like this, but you certainly wouldn’t want to use one just for this one piece of functionality, as it adds nearly 100kb to your download unnecessarily! There are very small libraries (known as micro libraries) which do just focus on one element of functionality. For example, Events.js provides a cross-browser events system which would solve the problem. The 140medley library actually solves a few problems* besides event handling, but it’s tiny (we’re talking bytes rather than kilobytes) so it may be worth using it anyway.

However, my code is an addition to someone elses code, through this process we’ve actually added to the code drastically (though this has added further functionality). We’ve even added to the number of file requests made by separating out the styles (we could probably sort this out with even more code). While I originally wanted to keep things lean I’ve expanded the code, adding another library on top of this would prove even more weighty! For this reason I am going to keep my custom event handler set up as it is, (though I’ve just added a remove and an initiate function to make it a full implementation), I will make this available on GitHub when I finish this series of articles so other people can use this basic implementation, and if you’d prefer to use someone elses code instead, Nicholas Zakas has a great implementation too.

querySelector

The event handling issues are definitely the worst parts of the code to deal with where compatibility is concerned. But a much more common compatibility issue is the use of element.querySelector(). This function is brilliant for targeting any element on the page using CSS-style selectors. Without it we are limited to functions such as element.getElementById(), which only allows the targeting of one element at a time. As above, jQuery and MooTools have had this selector functionality built in for years. But again, adding a near 100kb file to your load just for this functionality is overkill, I would instead go back to recommending 140medley* which will sort this functionality out for you in less than a kilobyte.

I won’t be using that library here because again, it would add more to our code, and I’d have to include a copyright license to legally use the code. Because the operations are simple, I’m instead going to fall back to the lowest common denominator. I know that all the browsers that I need compatibility in have functions such as element.getElementById(), element.getElementsByClassName() and element.getElementsByTagName(). So I will simply rewrite my code to use these.

...
AutoScrobbler.prototype.addLatest = function() {

var tracks = document.querySelectorAll(".userSong");
//Can instead be written as
var tracks = document.getElementsByClassName("userSong");

... tracks[1].querySelector("input").value ...;
//Can instead be written as
... tracks[1].getElementsByTagName("input")[0].value ...;

...

... item.querySelector("input").value ...
//Can instead be written as
... item.getElementsByTagName("input")[0].value ...

...
... tracks[0].querySelector("input").value;
//Can instead be written as
... tracks[0].getElementsByTagName("input")[0].value;

...

item.querySelector("input").checked = true;
//Can instead be written as
item.getElementsByTagName("input")[0].checked = true;

...

}

I was using querySelector here “because I could” in most cases, rather than “because it was essential”. I wasn’t targeting anything very specific. Even if I had a couple of selectors in each querySelector, it would’ve easy to simply break these down into two steps. I have made things far more compatible, simply by taking time to rethink my code a little. This is the best type of compatibility refactoring, as it requires no extra code, just rewritten code to get further browser support.

We’ve made a few changes here, so I’ll post the full code below, just for reference.

/** AutoScrobbler is a bookmarklet/plugin which extends the Universal Scrobbler
*   web application, allowing automatic scrobbling of frequently updating track
*   lists such as radio stations.
*
*   This is the constructor, injecting the user controls and starting the first
*   scrobble.
*/
function AutoScrobbler() {

var userControls = "<div id=\"autoScrobbler\" class="auto-scrob-cont">\n"+
"<input id=\"autoScrobblerStart\" class="start" type=\"button\" value=\"Start auto-scrobbling\" /> | <input id=\"autoScrobblerStop\" class="stop" type=\"button\" value=\"Stop auto-scrobbling\" />\n"+
"<p class="status-report"><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p>\n"+
"</div>\n";
this.stylesUrl = "http://www.andrewhbridge.co.uk/bookmarklets/auto-scrobbler.css";
this.injectHTML(userControls, "#mainBody");
this.injectStyles();
this.startElm = document.getElementById("autoScrobblerStart");
this.stopElm = document.getElementById("autoScrobblerStop");
this.loopUID = -1;
this.lastTrackUID = undefined;
this.scrobbled = 0;
this.countReport = document.getElementById("autoScrobblerTracksScrobbled");
this.evtInit(["addLatest", "loadThenAdd", "start", "stop"]);
this.listen("addLatest", this.reportScrobble);
this.setNativeListener(this.startElm, 'click', this.start);
this.setNativeListener(this.stopElm, 'click', this.stop);
this.start();

}

/** Inject the css stylesheet into the <head> of the page.
*/
AutoScrobbler.prototype.injectStyles = function() {

var styles = document.createElement('SCRIPT');
styles.type = 'text/javascript';
styles.src = this.stylesUrl;
document.getElementsByTagName('head')[0].appendChild(styles);

}

/** Hashing function for event listener naming. Similar implementation to
*   Java’s hashCode function. Hash collisions are possible.
*
*   @param toHash The entity to hash (the function will attempt to convert
*                 any variable type to a string before hashing)
*   @return A number up to 11 digits long identifying the entity.
*/
AutoScrobbler.prototype.hasher = function(toHash) {

var hash = 0;
toHash = "" + toHash;
for (var i = 0; i < toHash.length; i++)

hash = ((hash << 5) - hash) + hash.charCodeAt(i);

}

/** Custom event initiator for events in AutoScrobbler.
*
*   @param eventName The name of the event. This may be an array of names.
*/
AutoScrobbler.prototype.evtInit = function(eventName) {

//Initialise the evtLstnrs object and the event register if it doesn't exist.
if (typeof this.evtLstnrs == "undefined")

this.evtLstnrs = {"_EVTLST_reg": {}};

if (typeof eventName == "object") {

for (var i = 0; i < eventName.length; i++) {

var event = eventName[i];
this.evtLstnrs[""+event] = [];

}

} else

this.evtLstnrs[""+eventName] = [];

}

/** Custom event listener for events in AutoScrobbler.
*
*   @param toWhat A string specifying which event to listen to.
*   @param fcn A function to call when the event happens.
*   @return A boolean value, true if the listener was successfully set. False
*           otherwise.
*/
AutoScrobbler.prototype.listen = function(toWhat, fcn) {

//Initialise the function register if not done already
if (typeof this.evtLstnrs._EVTLST_reg == "undefined")

this.evtLstnrs._EVTLST_reg = {};


if (this.evtLstnrs.hasOwnProperty(toWhat)) {

//Naming the function so we can remove it if required. Uses hasher.
var fcnName = this.hasher(fcn);

//Add the function to the list.
var event = this.evtLstnrs[toWhat];
event[event.length] = fcn;
this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName] = event.length;
return true;

} else

return false;

}

/** Custom event listener trigger for events in AutoScrobbler
*
*   @param what Which event has happened.
*/
AutoScrobbler.prototype.trigger = function (what) {

if (this.evtLstnrs.hasOwnProperty(what)) {

var event = this.evtLstnrs[what];
for (var i = 0; i < event.length; i++)

event[i]();

}

}

/** Custom event listener removal for events in AutoScrobbler
*
*   @param toWhat A string to specify which event to stop listening to.
*   @param fcn The function which should no longer be called.
*   @return A boolean value, true if removal was successful, false otherwise.
*/
AutoScrobbler.prototype.unlisten = function(toWhat, fcn) {

var fcnName = this.hasher(fcn);
if (this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName) {

var event = this.evtLstnrs[toWhat];
var fcnPos = this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];
event[fcnPos] = void(0);
delete this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];

return true;

}

return false;

}

/** Remove a native event listener regardless of the browser you're using.
*
*   @param eventElm The element that had been listening.
*   @param eventType The type of event that was being listened for.
*   @param callback The function that was set to trigger.
*   @return Undefined if successful, otherwise an exception will be thrown.
*/
AutoScrobbler.prototype.setNativeListener(eventElm, eventType, callback) {

if (eventElm.addEventListener)

eventElm.addEventListener(eventType, callback, false);

else if (eventElm.attachEvent)

eventElm.attachEvent('on' + eventType, callback);

else {

eventElm['on' + eventType] = callback;
if (typeof eventElm['on' + eventType] == "undefined")

throw "EventHandler Error: The event could not be set";

}

}

/** Set a native event listener regardless of the browser you're using.
*
*   @param eventElm The element to which the element to listen for, will involve.
*   @param eventType The type of event to listen for.
*   @param callback The function to call when the event happens.
*   @return Undefined if successful, otherwise an exception will be thrown.
*/
AutoScrobbler.prototype.removeNativeListener(eventElm, eventType, callback) {

if (eventElm.removeEventListener)

eventElm.removeEventListener(eventType, callback, false);

else if (eventElm.detachEvent)

eventElm.detachEvent('on' + eventType, callback);

else

eventElm['on' + eventType] = undefined;
if (typeof eventElm['on' + eventType] != "undefined")

throw "EventHandler Error: The event could not be removed";

}

/** A function which will inject a piece of HTML wrapped in a
*   <div> within any node on the page.
*
*   @param code The HTML code to inject.
*   @param where The node to inject it within.
*   @param extraParams An object which allows optional parameters
*   @param extraParams.outerDivId The id to be given to the wrapping <div>
*   @param extraParams.outerDivClass The class to be given to the wrapping <div>
*   @param extraParams.insertBeforeElm An element within the element given
*                                      in where, to insert the code before.
*/
AutoScrobbler.prototype.injectHTML(code, where, extraParams) {

if (typeof extraParams) {

if (extraParams.hasOwnProperty("outerDivId"))

var divId = extraParams.outerDivId;

if (extraParams.hasOwnProperty("outerDivClass"))

var divClass = extraParams.outerDivClass;

var insBefElm = (extraParams.hasOwnProperty("insertBeforeElm")) ? extraParams.insertBeforeElm : null;

}

var node = document.querySelector(where);
var elm = document.createElement('DIV');

if (divId)

elm.id = divId;

if (divClass)

elm.className = divClass

elm.innerHTML = code;
node.insertBefore(elm, insBefElm);

}

/** Starts the auto-scrobbler, scrobbles immediately and schedules an update
*   every 5 minutes.
*/
AutoScrobbler.prototype.start = function() {

this.loadThenAdd();
autoScrobbler.loopUID = setInterval(this.loadThenAdd, 300000);
autoScrobbler.start.disabled = true;
autoScrobbler.stop.disabled = false;

}

/** Stops the auto-scrobbler, ends the recurring update and zeros the required
*   variables.
*/
AutoScrobbler.prototype.stop = function() {

clearInterval(this.loopUID);
this.lastTrackUID = undefined;
this.loopUID = -1;
this.stop.disabled = true;
this.start.disabled = false;

}

/** Loads the new track list using Universal Scrobbler and schedules a scrobble
*   of the latest tracks 30 seconds afterwards.
*/
AutoScrobbler.prototype.loadThenAdd = function() {

doRadioSearch();
setTimeout(this.addLatest, 30000);

}

/** Selects all the tracks which have not been seen before and scrobbles them
*   using Universal Scrobbler.
*/
AutoScrobbler.prototype.addLatest = function() {

var tracks = document.getElementsByClassName(".userSong");
this.lastTrackUID = (typeof this.lastTrackUID == "undefined") ? tracks[1].getElementsByTagName("input")[0].value : this.lastTrackUID;

//Check every checkbox until the last seen track is recognised.
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.getElementsByTagName("input")[0].value == this.lastTrackUID) {

i = tracks.length;
this.lastTrackUID = tracks[0].getElementsByTagName("input")[0].value;

} else {

item.getElementsByTagName("input")[0].checked = true;
this.scrobbled++;

}

}
doUserScrobble();
this.trigger("addLatest");

}

/** Updates the user interfaces to reflect new scrobbles.
*/
AutoScrobbler.prototype.reportScrobble = function() {

this.countReport.innerHTML = this.scrobbled;

}

// Create a new instance of the AutoScrobbler.
autoScrobbler = new AutoScrobbler();

This concludes my series of articles on refactoring code, what we’re left with is a much higher quality of code, which is cleaner, more efficient and easier to add to in the future. I’ll round this series off with a final “teardown” article to review and add some helpful tips which you may want to follow as you refactor your own code.

* It should be noted that, while 140medley does a brilliant job and will definitely solve these compatibility options, it won’t check to see if the feature is already natively in the browser. This means modern browsers will be using a workaround needlessly.

Refactoring Code in to an Object-Oriented Paradigm #5: Cleaning Up Our Messy User Controls

This is article number 5 of a series of article, which simply documents my refactoring of some code I originally wrote (pretty poorly) last year. To get the gist of what’s going on. Check the original post – Refactoring Code in to an Object-Oriented Paradigm.

Cleaning Up Our Messy User Controls

We’re pretty close to being done here in terms of refactoring our Javascript. But the Auto-Scrobbler has a UI, an incredibly simple UI, but it still has one, and because this is a bookmarklet for a third-party website, the only way I can show this UI is by injecting the HTML on to the page. This not always the best way to do things, often it’s suggested that just the data is entered into pre-existing HTML on the page, but in this case it’s about the only thing we can do. But there are still best practices that come with this.

Here’s the current HTML that we inject into the page (post-injection):

<div id="autoScrobbler" style="background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;"><input id="autoScrobblerStart" type="button" value="Start auto-scrobbling" onclick="autoScrobbler.start();" /> | <input id="autoScrobblerStop" type="button" value="Stop auto-scrobbling" onclick="autoScrobbler.stop();" /><p><span id="autoScrobblerScrobbleCount">0</span> tracks scrobbled</p></div>

This is a horribly messy bit of HTML, having originally written this code quickly I haven’t even included any structure, it’s all just one flowing line. To make things worse I am using inline styles and, worse still, inline event handlers. The way I’m injecting the code isn’t great either, I’m adding it to the end of another element, which means the styles are liable to change and possibly break the UI.

Structure

That’s actually a lot of stuff to turn around, so we’ll just start with the structure. To get some structuring, we need to use the new line character at the end of each line (“\n” in most languages) in our Javascript. Like so:

var userControls = "<div id=\"autoScrobbler\" style=\"background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;\">\n"+
"<input id=\"autoScrobblerStart\" type=\"button\" value=\"Start auto-scrobbling\" onclick=\"autoScrobbler.start();\" /> | <input id=\"autoScrobblerStop\" type=\"button\" value=\"Stop auto-scrobbling\" onclick=\"autoScrobbler.stop();\" />\n"+
"<p><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p>\n"+
"</div>\n";

Which should inject something like this:

<div id="autoScrobbler" style="background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;">

<input id="autoScrobblerStart" type="button" value="Start auto-scrobbling" onclick="autoScrobbler.start();" /> | <input id="autoScrobblerStop" type="button" value="Stop auto-scrobbling" onclick="autoScrobbler.stop();" />

<p><span id="autoScrobblerScrobbleCount">0</span> tracks scrobbled</p>

</div>

Better, but those inline styles and event handlers are still making things messy.

Styling

We’ll solve the inline styles first.

It should first be mentioned that those inline styles aren’t just messy but also frustrating. Inline styles are generally the most specific you can get with CSS to styling elements, which makes sense, by putting styles inline, they will only ever be applied to that element unlike any other CSS that you can apply. But what it means is that another developers, or even I, myself trying to extend the bookmarklet in some way, won’t be able to alter the styling in any way without either changing the original source code or using “!important” after my rule. Using these rules can get messy because it’s only going to lead down a messy road of adding secondary or even tertiary “!important” rules to allow developers to get the result they’d like.

So we won’t even start going down that route, we’ll instead include a separate stylesheet with these styling rules. This should look something like…

.auto-scrob-cont {

position:fixed;
bottom: 0;
width: inherit;
min-height: 50px;
background: #FFFFFF;
border-top: 1px solid #000000;
border-left: 1px solid #000000;

}

Simple enough, and to load it in, we’ll use a similar function to the one that’s used to load in the bookmarklet in the first place (which is based on some of the code used in an article at betterexplained.com):

/** Inject the css stylesheet into the <head> of the page.
*/
AutoScrobbler.prototype.injectStyles = function() {

var styles = document.createElement('SCRIPT');
styles.type = 'text/javascript';
styles.src = this.stylesUrl;
document.getElementsByTagName('head')[0].appendChild(styles);

}

We’ll add this into our javascript, but this will essentially call a second file with the CSS above. This now means that there’s a certain flexibility, I can have several instances of the UI (maybe showing different bits of information for instance) around the page. To do this I’ve added to classes to each of the main HTML elements, as using the ID attribute to select the elements in CSS would eliminate that possibility of having several instances, it also means we can have more general names for elements, such as “start” and “stop”.

<div id="autoScrobbler" class="auto-scrob-cont">

<input id="autoScrobblerStart" class="start" type="button" value="Start auto-scrobbling" onclick="autoScrobbler.start();" /> | <input id="autoScrobblerStop" class="stop" type="button" value="Stop auto-scrobbling" onclick="autoScrobbler.stop();" />

<p class="status-report"><span id="autoScrobblerScrobbleCount">0</span> tracks scrobbled</p>

</div>

Event Handling

Next we’ll fix those inline event handlers. Along with making the HTML more messy, inline event handlers are an old method of reacting to different events, it goes against unobtrusive javascript because we’re pushing javascript into the HTML, rather than keeping each language separate.

The way we do this is with event handlers, in the previous article I actually wrote a custom event handler mechanism to side-step having to deal with the compatibility issues which come with using event handlers (which are only doubled when trying to create custom events like I was previously). Here though, we can’t get away from using them if we want to improve our code. My gripe with eventListeners is compatibility, but as that’s the title of the next article, I’ll continue side-stepping the compatibility issues which exist. For now, we’ll just implement the “modern browser” method, but I’ll put it into a new function so we can address the compatibility issues more easily later.

/** Set an event listener regardless of the browser you're using.
*
*   @param eventElm The element to which the element to listen for, will involve.
*   @param eventType The type of event to listen for.
*   @param callback The function to call when the event happens.
*   @return True if listener was set successfully, false otherwise.
*/
AutoScrobbler.prototype.setNativeListener(eventElm, eventType, callback) {

eventElm.addEventListener(eventType, callback, false);

}

//Implemented like this
this.setNativeListener(this.startElm, ‘click’, this.start);

Okay so now with all that stripped out, we have some much cleaner HTML. It’s far easier to read than before, and we can assume what kind of thing the complementing Javascript will do to the elements.

<div id="autoScrobbler" class="auto-scrob-cont">

<input id="autoScrobblerStart" class="start" type="button" value="Start auto-scrobbling" /> | <input id="autoScrobblerStop" class="stop" type="button" value="Stop auto-scrobbling" />

<p class="status-report"><span id="autoScrobblerScrobbleCount">0</span> tracks scrobbled</p>

</div>

Code Injection

My final stage for this article is actually moving back to the Javascript code (I did say we were “nearly” there with it). As I said in the introduction to the article, the injection method I use is fine, but not perfect, it inserts it into another element on the page, which is dangerous, not only could that element be deleted, but the styles could change or even the structure of the page could change! It relies on the external code too heavily when it needn’t.

I’ll make an HTML injection method for the class, and I’ll make it pretty generic so that any HTML can be injected onto the page. This means that it’s a bit future-proofed, if we want to inject several bits of UI on the page, that facility is there!

/** A function which will inject a piece of HTML wrapped in a
*   <div> within any node on the page.
*
*   @param code The HTML code to inject.
*   @param where The node to inject it within.
*   @param extraParams An object which allows optional parameters
*   @param extraParams.outerDivId The id to be given to the wrapping <div>
*   @param extraParams.outerDivClass The class to be given to the wrapping <div>
*   @param extraParams.insertBeforeElm An element within the element given
*                                      in where, to insert the code before.
*/
AutoScrobbler.prototype.injectHTML(code, where, extraParams) {

if (typeof extraParams) {

if (extraParams.hasOwnProperty("outerDivId"))

var divId = extraParams.outerDivId;

if (extraParams.hasOwnProperty("outerDivClass"))

var divClass = extraParams.outerDivClass;

var insBefElm = (extraParams.hasOwnProperty("insertBeforeElm")) ? extraParams.insertBeforeElm : null;

}

var node = document.querySelector(where);
var elm = document.createElement('DIV');

if (divId)

elm.id = divId;

if (divClass)

elm.className = divClass

elm.innerHTML = code;
node.insertBefore(elm, insBefElm);

}

//To implement we use
this.injectHTML(userControls, "#mainBody");

Okay, we now have clean HTML, CSS, event handling and code injection. Just for reference, here’s our full code.

/** AutoScrobbler is a bookmarklet/plugin which extends the Universal Scrobbler
*   web application, allowing automatic scrobbling of frequently updating track
*   lists such as radio stations.
*
*   This is the constructor, injecting the user controls and starting the first
*   scrobble.
*/
function AutoScrobbler() {

var userControls = "<div id=\"autoScrobbler\" class="auto-scrob-cont">\n"+
"<input id=\"autoScrobblerStart\" class="start" type=\"button\" value=\"Start auto-scrobbling\" /> | <input id=\"autoScrobblerStop\" class="stop" type=\"button\" value=\"Stop auto-scrobbling\" />\n"+
"<p class="status-report"><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p>\n"+
"</div>\n";
this.stylesUrl = "http://www.andrewhbridge.co.uk/bookmarklets/auto-scrobbler.css";
this.injectHTML(userControls, "#mainBody");
this.injectStyles();
this.startElm = document.getElementById("autoScrobblerStart");
this.stopElm = document.getElementById("autoScrobblerStop");
this.loopUID = -1;
this.lastTrackUID = undefined;
this.scrobbled = 0;
this.countReport = document.getElementById("autoScrobblerTracksScrobbled");
this.evtInit(["addLatest", "loadThenAdd", "start", "stop"]);
this.listen("addLatest", this.reportScrobble);
this.setNativeListener(this.startElm, 'click', this.start);
this.setNativeListener(this.stopElm, 'click', this.stop);
this.start();

}

/** Inject the css stylesheet into the <head> of the page.
*/
AutoScrobbler.prototype.injectStyles = function() {

var styles = document.createElement('SCRIPT');
styles.type = 'text/javascript';
styles.src = this.stylesUrl;
document.getElementsByTagName('head')[0].appendChild(styles);

}

/** Hashing function for event listener naming. Similar implementation to
*   Java’s hashCode function. Hash collisions are possible.
*
*   @param toHash The entity to hash (the function will attempt to convert
*                 any variable type to a string before hashing)
*   @return A number up to 11 digits long identifying the entity.
*/
AutoScrobbler.prototype.hasher = function(toHash) {

var hash = 0;
toHash = "" + toHash;
for (var i = 0; i < toHash.length; i++)

hash = ((hash << 5) - hash) + hash.charCodeAt(i);

}

/** Custom event initiator for events in AutoScrobbler.
*
*   @param eventName The name of the event. This may be an array of names.
*/
AutoScrobbler.prototype.evtInit = function(eventName) {

//Initialise the evtLstnrs object and the event register if it doesn't exist.
if (typeof this.evtLstnrs == "undefined")

this.evtLstnrs = {"_EVTLST_reg": {}};

if (typeof eventName == "object") {

for (var i = 0; i < eventName.length; i++) {

var event = eventName[i];
this.evtLstnrs[""+event] = [];

}

} else

this.evtLstnrs[""+eventName] = [];

}

/** Custom event listener for events in AutoScrobbler.
*
*   @param toWhat A string specifying which event to listen to.
*   @param fcn A function to call when the event happens.
*   @return A boolean value, true if the listener was successfully set. False
*           otherwise.
*/
AutoScrobbler.prototype.listen = function(toWhat, fcn) {

//Initialise the function register if not done already
if (typeof this.evtLstnrs._EVTLST_reg == "undefined")

this.evtLstnrs._EVTLST_reg = {};


if (this.evtLstnrs.hasOwnProperty(toWhat)) {

//Naming the function so we can remove it if required. Uses hasher.
var fcnName = this.hasher(fcn);

//Add the function to the list.
var event = this.evtLstnrs[toWhat];
event[event.length] = fcn;
this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName] = event.length;
return true;

} else

return false;

}

/** Custom event listener trigger for events in AutoScrobbler
*
*   @param what Which event has happened.
*/
AutoScrobbler.prototype.trigger = function (what) {

if (this.evtLstnrs.hasOwnProperty(what)) {

var event = this.evtLstnrs[what];
for (var i = 0; i < event.length; i++)

event[i]();

}

}

/** Custom event listener removal for events in AutoScrobbler
*
*   @param toWhat A string to specify which event to stop listening to.
*   @param fcn The function which should no longer be called.
*   @return A boolean value, true if removal was successful, false otherwise.
*/
AutoScrobbler.prototype.unlisten = function(toWhat, fcn) {

var fcnName = this.hasher(fcn);
if (this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName) {

var event = this.evtLstnrs[toWhat];
var fcnPos = this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];
event[fcnPos] = void(0);
delete this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];

return true;

}

return false;

}

/** Set a native event listener (eventually regardless of the browser you're using).
*
*   @param eventElm The element to which the element to listen for, will involve.
*   @param eventType The type of event to listen for.
*   @param callback The function to call when the event happens.
*   @return True if listener was set successfully, false otherwise.
*/
AutoScrobbler.prototype.setNativeListener(eventElm, eventType, callback) {

eventElm.addEventListener(eventType, callback, false);

}

/** A function which will inject a piece of HTML wrapped in a
*   <div> within any node on the page.
*
*   @param code The HTML code to inject.
*   @param where The node to inject it within.
*   @param extraParams An object which allows optional parameters
*   @param extraParams.outerDivId The id to be given to the wrapping <div>
*   @param extraParams.outerDivClass The class to be given to the wrapping <div>
*   @param extraParams.insertBeforeElm An element within the element given
*                                      in where, to insert the code before.
*/
AutoScrobbler.prototype.injectHTML(code, where, extraParams) {

if (typeof extraParams) {

if (extraParams.hasOwnProperty("outerDivId"))

var divId = extraParams.outerDivId;

if (extraParams.hasOwnProperty("outerDivClass"))

var divClass = extraParams.outerDivClass;

var insBefElm = (extraParams.hasOwnProperty("insertBeforeElm")) ? extraParams.insertBeforeElm : null;

}

var node = document.querySelector(where);
var elm = document.createElement('DIV');

if (divId)

elm.id = divId;

if (divClass)

elm.className = divClass

elm.innerHTML = code;
node.insertBefore(elm, insBefElm);

}

/** Starts the auto-scrobbler, scrobbles immediately and schedules an update
*   every 5 minutes.
*/
AutoScrobbler.prototype.start = function() {

this.loadThenAdd();
autoScrobbler.loopUID = setInterval(this.loadThenAdd, 300000);
autoScrobbler.start.disabled = true;
autoScrobbler.stop.disabled = false;

}

/** Stops the auto-scrobbler, ends the recurring update and zeros the required
*   variables.
*/
AutoScrobbler.prototype.stop = function() {

clearInterval(this.loopUID);
this.lastTrackUID = undefined;
this.loopUID = -1;
this.stop.disabled = true;
this.start.disabled = false;

}

/** Loads the new track list using Universal Scrobbler and schedules a scrobble
*   of the latest tracks 30 seconds afterwards.
*/
AutoScrobbler.prototype.loadThenAdd = function() {

doRadioSearch();
setTimeout(this.addLatest, 30000);

}

/** Selects all the tracks which have not been seen before and scrobbles them
*   using Universal Scrobbler.
*/
AutoScrobbler.prototype.addLatest = function() {

var tracks = document.querySelectorAll(".userSong");
this.lastTrackUID = (typeof this.lastTrackUID == "undefined") ? tracks[1].querySelector("input").value : this.lastTrackUID;

//Check every checkbox until the last seen track is recognised.
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.querySelector("input").value == this.lastTrackUID) {

i = tracks.length;
this.lastTrackUID = tracks[0].querySelector("input").value;

} else {

item.querySelector("input").checked = true;
this.scrobbled++;

}

}
doUserScrobble();
this.trigger("addLatest");

}

/** Updates the user interfaces to reflect new scrobbles.
*/
AutoScrobbler.prototype.reportScrobble = function() {

this.countReport.innerHTML = this.scrobbled;

}

// Create a new instance of the AutoScrobbler.
autoScrobbler = new AutoScrobbler();

That’s it for this article, next we’ll be looking at achieving as full as possible compatibility.

Refactoring Code in to an Object-Oriented Paradigm #4: Extendible, Readable, Descriptive

This is article number 4 of a series of article, which simply document my refactoring of some code I originally wrote (pretty poorly) last year. To get the jist of what’s going on. Check the original post – Refactoring Code in to an Object-Oriented Paradigm.

Making code Extensive, Readable and Descriptive

We’re nearly there with the Javascript, the worst has been cleaned up generally. Now we’re getting on to being developer-friendly, which is pretty important, you want people to extend your program. Even if you want to have control over the way they interface with your program, you still want people to interface with it somehow.

There are a few ways you can make things easier for developers, we’ve reorganised the code in the previous step, but it’s an important step, just by putting the constructor code up the top, you can suddenly look at the code and understand it just by scrolling down the page once. At least, that’s the idea, but sometimes it doesn’t hurt to actually explain what a method is doing, or what a specifically tricky part of the code does. This is why we use comments above each function, and in places actually within the code.

Finally, while it’s not quite keeping exactly the same functionality, we’re going to implement a system which allows other functions to listen for certain events. This means there’s a properly implemented way of extending our code. To ease our compatibility stage at the end, I’m going to use a custom function for custom event handling, that I’ve developed myself.

/** AutoScrobbler is a bookmarklet/plugin which extends the Universal Scrobbler
*   web application, allowing automatic scrobbling of frequently updating track
*   lists such as radio stations.
*
*   This is the constructor, injecting the user controls and starting the first
*   scrobble.
*/
function AutoScrobbler() {

var userControls = "<div id=\"autoScrobbler\" style=\"background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;\">"+
"<input id=\"autoScrobblerStart\" type=\"button\" value=\"Start auto-scrobbling\" onclick=\"autoScrobbler.start();\" /> | <input id=\"autoScrobblerStop\" type=\"button\" value=\"Stop auto-scrobbling\" onclick=\"autoScrobbler.stop();\" />"+
"<p><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p>"+
"</div>";
document.querySelector("#disclaimersContainer").innerHTML += userControls;
this.startElm = document.getElementById("autoScrobblerStart");
this.stopElm = document.getElementById("autoScrobblerStop");
this.loopUID = -1;
this.lastTrackUID = undefined;
this.scrobbled = 0;
this.countReport = document.getElementById("autoScrobblerTracksScrobbled");
this.evtInit(["addLatest", "loadThenAdd", "start", "stop"]);
this.listen("addLatest", this.reportScrobble);
this.start();

}

/** Hashing function for event listener naming. Similar implementation to
*   Java's hashCode function. Hash collisions are possible.
*
*   @param toHash The entity to hash (the function will attempt to convert
*                 any variable type to a string before hashing)
*   @return A number up to 11 digits long identifying the entity.
*/
AutoScrobbler.prototype.hasher = function(toHash) {

var hash = 0;
toHash = "" + toHash;
for (var i = 0; i < toHash.length; i++)

hash = ((hash << 5) - hash) + hash.charCodeAt(i);

}

/** Custom event initiator for events in AutoScrobbler.
*
*   @param eventName The name of the event. This may be an array of names.
*/
AutoScrobbler.prototype.evtInit = function(eventName) {

//Initialise the evtLstnrs object and the event register if it doesn't exist.
if (typeof this.evtLstnrs == "undefined")

this.evtLstnrs = {"_EVTLST_reg": {}};

if (typeof eventName == "object") {

for (var i = 0; i < eventName.length; i++) {

var event = eventName[i];
this.evtLstnrs[""+event] = [];

}

} else

this.evtLstnrs[""+eventName] = [];

}

/** Custom event listener for events in AutoScrobbler.
*
*   @param toWhat A string specifying which event to listen to.
*   @param fcn A function to call when the event happens.
*   @return A boolean value, true if the listener was successfully set. False
*           otherwise.
*/
AutoScrobbler.prototype.listen = function(toWhat, fcn) {

if (this.evtLstnrs.hasOwnProperty(toWhat)) {

//Naming the function so we can remove it if required. Uses hasher.
var fcnName = this.hasher(fcn);

//Add the function to the list.
var event = this.evtLstnrs[toWhat];
event[event.length] = fcn;
this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName] = event.length;
return true;

} else

return false;

}

/** Custom event listener trigger for events in AutoScrobbler
*
*   @param what Which event has happened.
*/
AutoScrobbler.prototype.trigger = function (what) {

if (this.evtLstnrs.hasOwnProperty(what)) {

var event = this.evtLstnrs[what];
for (var i = 0; i < event.length; i++)

event[i]();

}

}

/** Custom event listener removal for events in AutoScrobbler
*
*   @param toWhat A string to specify which event to stop listening to.
*   @param fcn The function which should no longer be called.
*   @return A boolean value, true if removal was successful, false otherwise.
*/
AutoScrobbler.prototype.unlisten = function(toWhat, fcn) {

var fcnName = this.hasher(fcn);
if (this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName) {

var event = this.evtLstnrs[toWhat];
var fcnPos = this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];
event[fcnPos] = void(0);
delete this.evtLstnrs._EVTLST_reg[toWhat+"->"+fcnName];

return true;

}

return false;

}

/** Starts the auto-scrobbler, scrobbles immediately and schedules an update
*   every 5 minutes.
*/
AutoScrobbler.prototype.start = function() {

this.loadThenAdd();
autoScrobbler.loopUID = setInterval(this.loadThenAdd, 300000);
autoScrobbler.start.disabled = true;
autoScrobbler.stop.disabled = false;

}

/** Stops the auto-scrobbler, ends the recurring update and zeros the required
*   variables.
*/
AutoScrobbler.prototype.stop = function() {

clearInterval(this.loopUID);
this.lastTrackUID = undefined;
this.loopUID = -1;
this.stop.disabled = true;
this.start.disabled = false;

}

/** Loads the new track list using Universal Scrobbler and schedules a scrobble
*   of the latest tracks 30 seconds afterwards.
*/
AutoScrobbler.prototype.loadThenAdd = function() {

doRadioSearch();
setTimeout(this.addLatest, 30000);

}

/** Selects all the tracks which have not been seen before and scrobbles them
*   using Universal Scrobbler.
*/
AutoScrobbler.prototype.addLatest = function() {

var tracks = document.querySelectorAll(".userSong");
this.lastTrackUID = (typeof this.lastTrackUID == "undefined") ? tracks[1].querySelector("input").value : this.lastTrackUID;

//Check every checkbox until the last seen track is recognised.
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.querySelector("input").value == this.lastTrackUID) {

i = tracks.length;
this.lastTrackUID = tracks[0].querySelector("input").value;

} else {

item.querySelector("input").checked = true;
this.scrobbled++;

}

}
doUserScrobble();
this.trigger("addLatest");

}

/** Updates the user interfaces to reflect new scrobbles.
*/
AutoScrobbler.prototype.reportScrobble = function() {

this.countReport.innerHTML = this.scrobbled;

}

// Create a new instance of the AutoScrobbler.
autoScrobbler = new AutoScrobbler();

Reorganisation

There’s quite a few changes to the structure of the code here. You’ll notice that the code now reads in the same way as the code would be used. When you run the bookmarklet initially, you’d run the constructor code, which then calls start(), which in turn calls loadAndAdd() and so on.

The only exception to this is the stop() command, starting and finishing methods like these are often grouped at the top of the code, to make it clear to the programmer what the basic functions are. As stop() doesn’t use any method which hasn’t been seen further up the code, it also won’t confuse the programmer, as it’s clear how the method works, without knowing anymore about the code.

Comments

You’ll also notice that each method has a comment to go with it, and in certain places, helpful hints have been added in the code. This can be very important for your code, as your way of thinking about the architecture of the code is very rarely the same as someone else’s way of thinking. Comments communicate in natural language, what the function or method’s overall goal is, they should generally not dissect the code and describe each operation (this can be done within the code if really required).

The way that I’ve written them is one of several loose standards for commenting. This style is similar to the way comments are written in Java. It is often useful to write comments in a common standard, as developers will be able to understand them more immediately, but other programs may also be able to understand them. There are many programs which have been written to interpret comments like these into formatted technical documentation (The type that you’d see on the MDN, PHP.net or Oracle Docs), saving you a great deal of time and effort and being a neighbourly developer to anyone that wants to look, use or edit your code.

I’ve used a few of these programs myself, I’ve had most success with Natural Docs (for Javascript, though it has support for other languages), PHP Documentor (A bit of a fiddle, but made great PHP documentation) and Oracle’s own Javadoc (For Java of course).

Extensibility

You will also notice that I’ve added three new methods, two of which help me and other developers extend the code in a documented way. The alternative to this would be for other developers to rely on the variables and functions to remain the same in name and the action they perform, this is a bad idea. The two new functions (listen() and trigger()), allow any other functions to hook on to the function/event that they want to know about and have a function called when the function/event happens.

By using this system, events can be named independently of the internal workings of the code, which means more stability for us and other developers.

The third function is a demonstration of this implemented extensibility, in the constructor method, I start “listening” for the “addLatest” event with reportScrobble(), which for now just signifies that the addLatest() method has run and calls reportScrobble() to update any user interface. The event gets triggered simply by calling this.trigger(“addLatest”).

I haven’t included a “removeListen” function here, as it gets quite complicated in pure Javascript, but this would also be possible, and would allow code to stop listening to our events.

Note (07/02/2013)

I have since added a remove and a evtInit function to the event listener mechanism (which I have added above in the code). It has slightly complicated the implementation and you’ll notice a new hasher function which is used to name functions in a way that they can be identified later. I decided that it’d be better to have a custom implementation which was at least complete (often an issue with custom implementations). The rest of the code here is unchanged (except for the initiation of events), I have in fact refactored my own code, making sure that all existing functionality remains.

I have further researched native implementations for custom events, which I haven’t used because of issues with compatibility (the final article in the series will have more on this). If you are interested in using these native methods, Sitepoint has a pretty good article for the basics.

In terms of refactoring our Javascript, we’re just about done. We now have a far cleaner implementation, which is now a lot more developer friendly. But for all the code, there’s a user interface, and everything about it has been neglected thus far, so next we’ll be looking at Cleaning Up Your Messy User Controls.

Refactoring Code in to an Object-Oriented Paradigm #3: Moving to Object-Oriented code patterns

This is article number 3 of a series of article, which simply documents my refactoring of some code I originally wrote (pretty poorly) last year. To get the gist of what’s going on. Check the original post – Refactoring Code in to an Object-Oriented Paradigm.

Moving Code to Object-Oriented Code Patterns

So now we have our group of variables under the name autoScrobbler, I said earlier that we could also include functions under this group. This is quite a good way of going, because at the moment, we’re taking up the function name stop() globally, any function that calls stop when this code is loaded will run our function.

This can cause problems for other code and ours. For other programs, it may be trying to stop something else from running, when it calls stop(), it won’t have stopped what it expects to. For our programs, that means that the other program has just stopped the auto-scrobbler when the user hasn’t asked it to!

The other nice thing about putting our functions under the same group, is that we don’t need to rely on a name for the group. If we decide that autoScrobbler is a bad name for the group later down the line, that’s fine we can change the name and everything will still work.

function AutoScrobbler() {

var userControls = "<div id=\"autoScrobbler\" style=\"background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;\">"+
"<input id=\"autoScrobblerStart\" type=\"button\" value=\"Start auto-scrobbling\" onclick=\"autoScrobbler.start();\" /> | <input id=\"autoScrobblerStop\" type=\"button\" value=\"Stop auto-scrobbling\" onclick=\"autoScrobbler.stop();\" />"+
"<p><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p>"+
"</div>";
document.querySelector("#disclaimersContainer").innerHTML += userControls;
this.startElm = document.getElementById("autoScrobblerStart");
this.stopElm = document.getElementById("autoScrobblerStop");
this.loopUID = -1;
this.lastTrackUID = undefined;
this.scrobbled = 0;
this.countReport = document.getElementById("autoScrobblerTracksScrobbled");
this.start();

}

AutoScrobbler.prototype.addLatest = function() {

var tracks = document.querySelectorAll(".userSong");
this.lastTrackUID = (typeof this.lastTrackUID == "undefined") ? tracks[1].querySelector("input").value : this.lastTrackUID;
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.querySelector("input").value == this.lastTrackUID) {

i = tracks.length;
this.lastTrackUID = tracks[0].querySelector("input").value;

} else {

item.querySelector("input").checked = true;
this.scrobbled++;

}

}
doUserScrobble();
this.countReport.innerHTML = this.scrobbled;

}


AutoScrobbler.prototype.loadThenAdd = function() {

doRadioSearch();
setTimeout(this.addLatest, 30000);

}


AutoScrobbler.prototype.start = function() {

this.loadThenAdd();
autoScrobbler.loopUID = setInterval(this.loadThenAdd, 300000);
autoScrobbler.start.disabled = true;
autoScrobbler.stop.disabled = false;

}


AutoScrobbler.prototype.stop = function() {

clearInterval(this.loopUID);
this.lastTrackUID = undefined;
this.loopUID = -1;
this.stop.disabled = true;
this.start.disabled = false;

}

autoScrobbler = new AutoScrobbler();

This time, we’ve changed quite a lot. I’ll go through each of the changes.

Firstly, you’ll notice that each of the function headers (originally function addLatest() { and now AutoScrobbler.prototype.addLatest = function() {), have been changed. This is the best way of defining functions under the group that we grouped our variables under in the previous stage. There are other ways of doing this, but this generally has performance advantages. I’ve spoken about using prototype before, so if you’re interested in that, you should have a look at my post on using prototype.

Note (07/02/2013)

On reviewing the code for later articles, it has become clear that I have fallen into a classic “gotcha” or moving these functions into the same group as our variables.

Until I edited the code today, we had both a variable called start and a function called start(), when this code was run, the function (declared before the constructor function is called) is overwritten by the variable. So there is no start() function to call!

I have now corrected this, renaming the variables startElm and stopElm respectively. This is definitely something to check for if you are getting TypeErrors on running your code!

Secondly, you’ll notice that all references to autoScrobbler have been removed (except for one at the end). This is because everything is now in the same group of variables and functions, and we can use that third scope that we created in the previous stage without having to reference the name of the group explicitly, functions that are within the same group as variables can access these variables with the this keyword.

Next, you’ll notice that I’ve put all the code that was at the bottom at the top, and wrapped it in a function definition. This function header hasn’t been changed unlike the rest of the functions. This is part of the Object-Oriented style of coding. All this code was used to set up the variables, creating the user controls and essentially getting the program going. In Object-Oriented coding, this code goes in a constructor function (functions are often referred to as methods in Object-Oriented coding). Moving it to the top does nothing to the way in which the program runs, but is a more logical to have the code, because this is the code that runs to set up this group of variables and functions.

Finally, we’ve added a last line at the end, if you look at our previous stage you’ll see we have a similar line.

autoScrobbler = new Object();

In this stage we exchange the word Object for AutoScrobbler. In our previous stage, new Object() (as far as we need to know) set up our group to add variables, data and functions to. Using new AutoScrobbler() does the same setting up of the group, plus running any code in the constructor function.

A side effect of this is that we can create several of these groups, all of which can run at the same time and not interfere with each other. I said earlier that this probably isn’t something we want with our example, but it means that we could now, if we wanted, auto-scrobble two radio stations at a time.*

That’s it for this article, next we’ll be looking at extending, improving readability and describing code.

* Okay, not quite, if we tried this, the user controls would go a bit screwy, but the Javascript would not, the two would run independently of each other without an issue!

Refactoring Code in to an Object-Oriented Paradigm #2: Objects

This is article number 2 of a series of article, which simply documents my refactoring of some code I originally wrote (pretty poorly) last year. To get the gist of what’s going on. Check the original post – Refactoring Code in to an Object-Oriented Paradigm.

Using Objects

Javascript (and many other languages) has a variable type which, at it’s very simplest, will group other variables, data, even functions together. Usually these should be related to each other. We’ll start by grouping all of those global variables together, and for any variables which don’t need any greater scope than the local scope of that function, we’ll make sure they’re all defined with the var keyword. (I’ve stripped out the top comment for now)

function addLatest() {

var tracks = document.querySelectorAll(".userSong");
autoScrobbler.lastTrackUID = (typeof autoScrobbler.lastTrackUID == "undefined") ? tracks[1].querySelector("input").value : autoScrobbler.lastTrackUID;
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.querySelector("input").value == autoScrobbler.lastTrackUID) {

i = tracks.length;
autoScrobbler.lastTrackUID = tracks[0].querySelector("input").value;

} else {

item.querySelector("input").checked = true;
autoScrobbler.scrobbled++;

}

}
doUserScrobble();
autoScrobbler.countReport.innerHTML = autoScrobbler.scrobbled;

}


function loadThenAdd() {

doRadioSearch();
setTimeout(addLatest, 30000);

}


function start() {

loadThenAdd();
autoScrobbler.loopUID = setInterval(loadThenAdd, 300000);
autoScrobbler.start.disabled = true;
autoScrobbler.stop.disabled = false;

}


function stop() {

clearInterval(autoScrobbler.loopUID);
autoScrobbler.lastTrackUID = undefined;
autoScrobbler.loopUID = -1;
autoScrobbler.stop.disabled = true;
autoScrobbler.start.disabled = false;

}


document.querySelector("#disclaimersContainer").innerHTML += "<div id=\"autoScrobbler\" style=\"background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;\"><input id=\"autoScrobblerStart\" type=\"button\" value=\"Start auto-scrobbling\" onclick=\"start();\" /> | <input id=\"autoScrobblerStop\" type=\"button\" value=\"Stop auto-scrobbling\" onclick=\"stop();\" /><p><span id=\"autoScrobblerScrobbleCount\">0</span> tracks scrobbled</p></div>";
autoScrobbler = new Object();
autoScrobbler.start = document.getElementById("autoScrobblerStart");
autoScrobbler.stop = document.getElementById("autoScrobblerStop");
autoScrobbler.loopUID = -1;
autoScrobbler.lastTrackUID = undefined;
autoScrobbler.scrobbled = 0;
autoScrobbler.countReport = document.getElementById("autoScrobblerTracksScrobbled");
start();

What we have now done, is grouped all the variables that we need to make the plugin work correctly, under one group, called autoScrobbler. This hasn’t completely solved the problem, all these variables are still accessible globally, however if someone else wants to use the global variable start, the variable that we’re using here won’t be affected, because it’s autoScrobbler object which is variable, so you have to reference autoScrobbler to access and of the variables within it.

If you think about this more, we have created a third scope, now we have our global scope (addLatest() references the doUserScrobble() function for instance, which is accessible globally), we have all the local scope (addLatest() references tracks, which is only accessible within that function, nothing else can access it) and we also have a selection of variables which can only be accessed if you reference autoScrobbler.[variable name]. This is our third scope, and you can also place functions into this group…

That’s it for this article, next we’ll be looking at fully Moving to Object-Oriented code patterns.

Refactoring Code in to an Object-Oriented Paradigm

A lot of people will probably know how to refactor code, and many probably know about Object-Oriented programming too. But this doesn’t apply to everyone, and some people may not even know where to start with refactoring code, even if they do understand the concept of OO.

In any case, I decided that I’d document my refactoring of some code I’ve previously written, partly so I could be a lot more meticulous about it (exposing your code to criticism makes you far more pedantic about each line), but also to maybe help someone struggling with the two ideas, or some of the issues that I currently have with my code.

I’m splitting it up into a series of articles, so that I can focus on each point of the refactoring process and make it clear what I’m doing and why I’m doing it! So I’ll start with the introduction, and the first stage of the process: introducing objects!

I’m using some Javascript code here, but much the article could be applied to refactoring code in any programming language.

The Current Code

The code I’m editing is the code used by the Auto-Scrobbler, here’s the source code we’re starting with:


/*
Auto Scrobbler for Universal Scrobbler
This needs to be cleaned up, the global variables and procedural
style is not perfect!
*/


function addLatest() {

tracks = document.querySelectorAll(".userSong");
lastValue = (typeof lastValue == "undefined") ? tracks[1].querySelector("input").value : lastValue;
for (var i = 0; i < tracks.length; i++) {

var item = tracks[i];
if (item.querySelector("input").value == lastValue) {

i = tracks.length;
lastValue = tracks[0].querySelector("input").value;

} else {

item.querySelector("input").checked = true;
autoScrobblerScrobbled++;

}

}
doUserScrobble();
autoScrobblerScrobbledElm.innerHTML = autoScrobbler.scrobbled;

}


function loadThenAdd() {

doRadioSearch();
setTimeout(addLatest, 30000);

}


function start() {

loadThenAdd();
document.autoScrobbleUID = setInterval(loadThenAdd, 300000);
autoScrobblerStart.disabled = true;
autoScrobblerStop.disabled = false;

}


function stop() {

clearInterval(document.autoScrobbleUID);
lastValue = undefined;
document.autoScrobbleUID = 0;
autoScrobblerStop.disabled = true;
autoScrobblerStart.disabled = false;

}


document.querySelector("#disclaimersContainer").innerHTML += "<div id=\"autoScrobbler\" style=\"background: #FFFFFF; border-top: 1px solid #000000; border-left: 1px solid #000000; position: fixed; bottom: 0; height: 50px; width: inherit;\"><input id=\"autoScrobblerStart\" type=\"button\" value=\"Start auto-scrobbling\" onclick=\"start();\" /> | <input id=\"autoScrobblerStop\" type=\"button\" value=\"Stop auto-scrobbling\" onclick=\"stop();\" /></div>";
autoScrobblerStart = document.getElementById("autoScrobblerStart");
autoScrobblerStop: = document.getElementById("autoScrobblerStop");
autoScrobblerScrobbled = 0;
autoScrobblerScrobbledElm = document.getElementById("autoScrobblerTracksScrobbled");
start();

Some Issues

Global Variables

The only way we can pass round data is by setting up half a dozen global variables. Privacy isn’t really an issue in Javascript, you can access anything, there’s no privacy (without a lot of hacking an memory hogging). But it’s more of an issue of efficiency, all the variables will fill up the browser memory, making the code pretty inefficient.

If nothing else, it just fills up the list of global variables, just for your piece of code. Which means other developers will have to work around your big mass of variables.

To add to this, I’ve done some weird tomfoolery where I wanted to ensure that I was setting a global variable, by inserting variables into the “document” variable, which usually holds all the elements in the current document. If anything this should’ve been done in the “window” variable, where global variables are set by default. This would mean I wouldn’t be playing about with document properties, which is very bad practise!

No Chance of Multiple Instances

It doesn’t apply to this piece of code so much, but you can’t run multiple instances of this code. If I wanted to automatically scrobble two radio stations simultaneously (though I’m not sure why I’d want to), I wouldn’t be able to in the code’s current state. This would be more of an issue for something like an element fading script. If it was written in the same way as the code above, you’d only be able to fade one element on the page – that’s not very useful!

Difficult to extend, especially for others!

Because of the way that the code is written, extra functionality is hard to implement, old methods need to be rewritten (sometimes completely) to add any sort of functionality. It’d be nice to fix this, even allow other plugins to work with mine.

The way it’s written isn’t the most intuitive either, there’s no real flow, and if you want to follow the code as a developer who’s never seen the code before, you’ll be getting RSI from scrolling up and down if the code gets any longer! What’s worse is that there’s just one comment, the rest of the code has no information about what each function does.

It uses inline CSS and inline event handling

The code currently  injects some HTML into the page, to give the user some control over the automatic scrobbling. I wrote the whole plugin in the Chrome console, and at the time, both inline CSS and inline event handling seemed like the quickest way to write things. However, in terms of best practise, unobtrusive Javascript and CSS efficiency, it isn’t a good option to write any styles or event handles inline.

Limited Compatibility

It would be lovely to run one function or line of code, and expect it to work with every browser on every platform. Unfortunately, that’s just not how things are. Older browsers had a far more primitive method of handling events, and IE chose it’s own special way of doing things as well (as usual). Further to that, the W3C standard is also entirely different, so we need three different lines of code just to do one thing, this is also true of other functions I use in my code. As part of the refactor, it’d be great to get this compatibility for more browsers.

Let’s fix this

Those issues aren’t nice to have in production code, they may have been fine when I was trying to whip up something as quick as possible, but I’ve now drawn a line under it, and revealed it to others. So the code needs to be refactored. Refactoring won’t add any functionality to the plugin, nor will it change the resulting product or the data that is passed through the code.

In fact, perfect code refactoring would leave an end-user with the exact same functional experience as they had before. The difference is in how many resources are required to run the program, how easy it will be to add to that program later and how well other developers will be able to understand the code if they should look at it.

We’ll start with those global variables, we still want to be able to access them in any scope…

Aside: Scope

“A scope is the context in which a variable name or other identifier is valid and can be used.”

I’ve been meaning to write an article on the scope of variables and the problems I’ve been tripped up by just by refactoring code poorly, unfortunately I haven’t been able to do that as of yet!

You’ll need an idea of scope for this series of articles to make any sense at all, so here’s why we need to deal with different scopes in our specific example:

  • In it’s current state, there are two scopes:
    • Variables which are first set with the var keyword have a local scope. This means that, for instance, in the addLatest() function, the variable item may only be accessed within that function. If the loadThenAdd() function referenced the variable “item“, an error would be thrown, as it would not exist (because loadThenAdd() is outside of addLatest() scope). 
    • Variables which are first set without the var keyword have a global scope. This means that the variable could be referenced and used anywhere. So the start() function can set a value to document.autoScrobbleUID, and the stop() function can use that value to stop the auto-scrobbling of tracks.
    • Unless you know that you want to be using data between several functions, or that you want to retain data after the function has finished running, you should always put variables in a local scope. If you don’t, you can find some very tricky problems will occur (suddenly, you’ll find yourself overwriting variables in other parts of your code and other parts never running because the conditions aren’t right). I have in fact broken this rule in addLatest(), where tracks is used only by addLatest() and it doesn’t need to be retained after the function finishes, but it isn’t defined/set using the var keyword before it.
  • We use global variables for a lot of the variables in this code, because they are  required by multiple functions.
  • We also use the setInterval() function in the code, and any code you run within it has it’s own scope (it wouldn’t be able to access any local variables in the start() function for instance), so this also requires something with more access than local variables to do it’s job.
  • Understanding scope can help to make your programs more efficient which will increase performance as well!
  • The refactoring of our code will add a third scope into the mix, which solves many of these problems.

So we need to offer these variables globally (or at least in a wider scope than the local scope of each function), without cluttering up the rest of the page.

I am now going to refactor the code in five stages, I will be writing an article dedicated to each.

Objects

Javascript (and many other languages) has a variable type which, at it’s very simplest, will group other variables, data, even functions together… Read how I introduced objects into the code

Moving to Object-Oriented code patterns

So now we have our group of variables under the name autoScrobbler, I said earlier that we could also include functions under this group… Read how I moved my code to a more Object-Oriented pattern

Extendible, Readable, Descriptive

We’re nearly there with the Javascript, the worst has been cleaned up generally. Now we’re getting on to being developer-friendly, which is pretty important, you want people to extend your program… Read how I begin making my code more developer friendly.

Cleaning Up Our Messy User Controls

We’re pretty close to being done here in terms of refactoring our Javascript. But the Auto-Scrobbler has a UI, an incredibly simple UI, but it still has one… Read how I clean up the messy UI code.

Compatibility