Learning OOP javascript - critique my class that creates a 'favourites' list

Question

I'm trying to improve my javascript coding style and want to start writing my code more object-oriented. The code below is getting a JSON-stringified list of locations from a cookie and appending a li to a list for each location. I would appreciate some helpful critique and any general pointers. I have some questions too:

1. Am I using self correctly? It feels like I'm over using it i.e. on function calls but when its not there I cannot access those functions. Some plugins I've used are written in this way and some are not, why is that?

2. If I wanted to use this as a static class, how can I code it differently instead of instantiating it?

I have experience with vb/c# if that helps when explaining things.

var favourites = function(list, settings) {
    var self = this;

    self.options = {
        maxitems: 5,
        cookiename: "fav-locations"
    };
    self._$list = $(list) || $();
    self._locations = [];

    //extend defaults with any settings
    $.extend(self.options, settings);

    //load the locations
    var init = function (aryLocations) {
        var index = 0;
        if (aryLocations) {
            for (index; index < aryLocations.length; index++) {
                self.add(aryLocations[index]);
            }

            self.loadLocations();
            self.bindRemoveButtons();
        }
    };

    self.add = function (loc) {
        if ((self._locations.length < self.options.maxitems) && self.find(loc)) {
            self._locations.push(loc);
            $.cookie(self.options.cookiename, JSON.stringify(self._locations), { path: '/' });
        }
        else {
            console.log('exceeded limit');
        }
    };

    self.remove = function (loc) {
        var location_index = self.find(loc);
        console.log(location_index);
        if (location_index > -1) {
            self._locations.splice(location_index, 1);
            $.cookie(self.options.cookiename, JSON.stringify(self._locations), { path: '/' });
            self.setCount();
        }
        else {
            console.log('location not found.');
        }
    };

    self.populateLocations = function () {
        self.clearList();

        $.each(self._locations, function (index, item) {

            //get this locations name and id - format here is: name|id
            var location = {
                name: item.split('|')[0] || '',
                id: item.split('|')[1] || 0
            };

            //create a new li element
            var $thisItem = $('<li/>').appendTo(self._$list[0]).addClass('wrap');

            $thisItem.data("loc", item);

            $thisItem.html('<button class="remove-icon">&times;</button> <a href="/home/search-handler.asp">' + location.name + '</a>');
        });
    };

    self.setCount = function () {
        //set count value
        var countelem = self._$list.prev().find('.count');
        if (countelem.length > 0) { countelem.text(self._locations.length); };
    };

    self.loadLocations = function () {
        self.populateLocations();
        self.setCount();
    };

    self.clearList = function () {
        $(self._$list).find('li').remove();
    };

    self.find = function (loc) {
        var location_index = -1;
        for (var index = 0; index < self._locations.length; index++) {
            if (self._locations[index] === loc) {
                location_index = index;
                break;
            }
        }
        return location_index;
    };

    //remove button click handler
    self.bindRemoveButtons = function () {
        $(document).on('click', '.remove-icon', function (e) {
            var elem = $(this).parent();
            e.preventDefault();
            elem.slideUp('fast', function () {
                self.remove(elem.data("loc"));
                elem.remove();
            });
        });
    };

    //init the list
    init(JSON.parse($.cookie(self.options.cookiename)));
};

var favLocs = new favourites("ul.favourites-list", { tester: 1 });

---

UPDATE: Using the feedback below I've refactored the code implementing as much as I can. The events are still there as I'm not sure how to resolve that one but I think the code has improved.

var Favourites = (function ($) {
    'use strict';

    //constructor
    function Favourites(list, settings) {
        this.options = {
            maxitems: 5,
            cookiename: "fav-locations",
            searchurl: null
        };
        this._$list = $(list) || $();
        this._locations = [];

        //extend defaults with any settings
        $.extend(this.options, settings);

        //add locations to the array
        var index = 0,
            locations = JSON.parse($.cookie(this.options.cookiename));

        for (index; index < locations.length; index++) {
            this.add(locations[index]);
        }

        this.loadLocations();
        this.bindRemoveButtons();
        this.bindLocationLinkHandler();
    }
    //methods
    Favourites.prototype = {
        add: function (loc) {
            if ((this._locations.length < this.options.maxitems) && this.find(loc)) {
                this._locations.push(loc);
                $.cookie(this.options.cookiename, JSON.stringify(this._locations), { path: '/' });
            } else {
                console.log("exceeded limit");
            }
        },
        remove: function (loc) {
            var location_index = this.find(loc);
            console.log(location_index);
            if (location_index > -1) {
                this._locations.splice(location_index, 1);
                $.cookie(this.options.cookiename, JSON.stringify(this._locations), { path: '/' });
                this.setCount();
            } else {
                console.log("location not found.");
            }
        },
        populateLocations: function () {
            var self = this;

            this.clearList();

            $.each(this._locations, function (item) {

                //get this locations name and id - format here is: name|id
                var location = {
                    name: item.split('|')[0] || '',
                    id: item.split('|')[1] || 0
                },
                    //create a new li element
                    $thisItem = $('<li/>').appendTo(self._$list[0]).addClass("wrap");

                $thisItem.data("location", location.name);
                $thisItem.data("location-id", location.id);

                $thisItem.html('<button class="remove-icon">&times;</button> <a href="' + self.options.searchurl + '" class="fav-link">' + location.name + '</a>');
            });
        },
        setCount: function () {
            //set count value
            var countelem = this._$list.prev().find('.count');
            if (countelem.length > 0) { countelem.text(this._locations.length); }
        },
        loadLocations: function () {

            if ($(".search-nav:visible .current").length > 0) {
                $(".favourites, .search-box").fadeIn(300);
            }

            this.populateLocations();
            this.setCount();
        },
        clearList: function () {
            $(this._$list).find('li').remove();
        },
        find: function (loc) {
            var location_index = -1,
                index = 0;
            for (index; index < this._locations.length; index++) {
                if (this._locations[index] === loc) {
                    location_index = index;
                    break;
                }
            }
            return location_index;
        },
        bindRemoveButtons: function () {
            var self = this;
            $(document).on('click', '.remove-icon', function (e) {
                var elem = $(this).parent();
                e.preventDefault();
                elem.slideUp('fast', function () {
                    self.remove(elem.data("location") + '|' + elem.data("location-id"));
                    elem.remove();
                });
            });
        },
        bindLocationLinkHandler: function () {
            var self = this;
            $(document).on("click", ".fav-link", function (e) {
                var elem = $(this).parent();

                self.forwardToSection(elem.data("location"), elem.data("location-id"), e);
            });
        },
        forwardToSection: function (loc, locid, event) {
            var section = $(".search-nav:visible .current a").text(),
                qs = '?loc_Location=' + encodeURI(loc) + '&loc_exactlocation=' + locid;

            if (section) {
                if (event) {
                    event.currentTarget.href = event.currentTarget.href + qs + '&searchSite=' + section.toLowerCase();
                } else {
                    window.location = this.options.searchurl + qs + '&searchSite=' + section.toLowerCase();
                }
            } else {
                e.preventDefault();
                alert("Please select a section above");
            }
        }
    };

    return Favourites;
}(jQuery));

var favLocs = new Favourites("ul.favourites-list", { searchurl: "/home/search-handler.asp" });

Answer 1

Some things to consider (after a first, superficial look at your code):

• Stick to the conventions: A constructor starts with an UpperCase, and is camelCased from their on. Favourites would be a better var name, because it signals a constructor, and requires the new keyword.
• self, though not a reserved keyword is used by some JS engines for particular purposes. using self can lead to unexpected behaviour. Perhaps consider using var that = this; instead.
• Use the prototype for methods, to avoid creating too many function objects.
• self isn't required all the time, but it's safer. You're not using it badly, you're using it, but don't seem to know what it's for.
• Binding event handlers (self.bindRemoveButtons) is not the job of an object, or any of its methods. It's not because you're using the new keyword, and methods, that you're writing good, OO code. Besides, JS is prototype-based, using traditional OO techniques in JS is like a barber, using a scythe to cut your hair...
• The init function needn't be declared for each instance, use a closure.
• You have no guarantee that $ will be the jQuery object, use a closure.

Now, some examples to help you on your way:

var Favourites = (function($)//pass jQ object to closure
{
    'use strict';
    var init = function()
    {
        //the init function here.
    };
    function Favourites(list, settings)
    {//properties here...
        this._$list = $(list) || $();
        init();//call closure function init declared above ^^
    }
    Favourites.prototype.setCount = function()
    {
        //methods belong to the prototype
    };
    return Favourites;//return reference to constructor
}(jQuery));

This is what a more complex constructor definition in JS looks like.

Update:
Your updated code looks good. Honestly. It's not perfect, but then, in programming, perfection is more of a subjective thing. Personally, I rarely, if not never write constructors. I just use object literals and the module pattern.

You've looked into closures. Great, I get the impression you get what they're for, but you don't yet see all the use-cases for them. For example the bindLocationLinkHandler method, and for that matter, all the DOM-related methods, contain $(document). What this code does, essentially is call the jQuery init funtion, and constructs a jQuery wrapper object around the document object. Why would you do this over and over? why not wrap all those prototype methods in a simple closure like this:

(function($doc)
{
    Favourites.prototype.bindLocationLinkHandler = function()
    {
        $doc.on('click', ...);
    };
}($(document));//pass jQ wrapped document here

This'll save you a few jQ init calls. Your methods are also binding the same handler. Functions are first-class object in JS, construct them once, and use them everywhere. Replace, for example:

$(document).on("click", ".fav-link", function (e)
{
    var elem = $(this).parent();
    self.forwardToSection(elem.data("location"), elem.data("location-id"), e);
});

with something like:

(function($doc)
{
    var self, handler = function(e)
    {
        var elem = $(this).parent();
        self.forwardToSection(elem.data("location"), elem.data("location-id"), e)
    };
    Favourites.prototype.bindLocationLinkHandler = function()
    {
        self = this;//DANGEROUS... won't always work
        $doc.on('click', '.fav-link', handler);
    };
}($(document)));

Of course, you may have spotted that, owing to there only being a single self closure var for all of the instances, the self reference won't be as reliable. It depends on whatever method you're calling and weather or not it matters if you're calling the method on the same object or not. Either way, this was just to show that you can use closures for anything, simple, primitive variables, as well as functions. In fact, you're doing just that in jQ all the time!:

$('li').each(function()
{//<-- jQ's each method accepts an function as argument
});

To close this rant (few, I'm all over the place), I do have 1 more thing I'd like to point out. I left it out of my initial answer because I don't want to spark a pointless, yet lively debate, but still... In for a penny, in for a pound:
If I'm honest, I'm not too keen on jQuery. There's nothing wrong with jQ, but it's just something that is used too much. If all you're doing is delegate a single event, and perform one or two ajax calls, you don't need jQ, in such a scenario, where you're only using 5% of a monolithic toolkit, you're better off writing a few more lines of code. The performance benefits are, often, noticeable.

In this case, one line in particular got up my nose. You're dabbling with OO JS. The point of OO is to contain certain sets of data and logic in manageable, clearly defined entities (objects). Your object Favourites depends on a toolkit that is anything but modular. An object that relies on an entire lib, just so the constructor can do what it needs to do is, IMHO, just plain silly.
Why not replace this line:

$.extend(this.options, settings);

With a simple loop:

for (var p in settings)
{
    this.options[p] = settings[p];
}

You can add checks for settings[p], to ensure you're not assigning a function object to this.options[p], include a line like if (this.options.hasOwnProperty(p)) to deal with overriding of certain default properties, making sure the new values are valid and so on.
You could work on this code a bit more, and avoid your Favourites objects to depend on jQ being loaded all together, simply by doing something like:

this._list = document.querySelectorAll(list);

Most jQ selectors work with querySelectorAll anyway, and querySelectorAll is supported by IE8, even, so on the X-browser front, you're not going to have too many issues either.
A way to delegate the click event X-browser isn't that hard, either:

var clickHandler = function(e)
{
    e = e || window.event;
    var target = e.target || e.srcElement;
    if (target.className.test(/\bremove-icon\b/))
    {//handle remove-icon class elements only
        e.returnValue = false;
        if (e.preventDefault) e.preventDefault();
        target.parentNode.parentNode.removeChild(target.parentNode);
    }
};
if (window.addEventListener)
{
    window.addEventListener('click', clickHandler, false);
}
else
{
    window.attachEvent('onclick', clickHandler);
}

Granted, this is a bit more verbose, but it's not that difficult, IMO. The slide effect will require a bit more work, granted, but not nearly as much as you might think. All in all, you're free to use jQ if you want, just know that whatever jQ does, vanillaJS can do, too. If you take the time, and put in the effort to learn how you can do those things in vanillaJS, you'll find that you'll learn a great deal more about how JS actually works, which will improve the quality of your code, even when you do decide to use jQ

Answer 2

Using self = this is actually a bad practice. You'll end up with zillions of self that will mean different things at different times, and the mess will eat you up. Sadly it is used in many tutorials just because people are lazy to come up with better names.

Give it meaningful names like user = this or car = this or whatever. So much more readable and helpful to whoever struggles to understand the code. Even the abstract myObject = this is already better.