Directory Searcher

This is about my first backbone.js application. It is a small directory that fetches data and presents it in a search-like UI.

Here is a working (minified) example.

Here is the file that handles everything

```
window.Directory = function() {
"use strict";

// Create the view and listen to main events
this.view = new this.AppView().on({
// Main functions that take care of searching and returning results for multiple items.
'Dir.parseForm': function() { this.parseForm() },
'Dir.parsedTheForm': function(selectedThings) { this.prepareResultsCollection(selectedThings) },
'Dir.resultsCollectionPrepared': function() { this.collection.fetch() },
'Dir.fetchingThings': function() {}, // Just here in case we need it
'Dir.doneFetching': function() { this.createArrayOfElements() },
'Dir.elementsReadyToInsert': function(elementsToInsert) { this.insertIntoTheDOM(elementsToInsert) }, // We're all done.

// Functions for handling when a single element wants to be viewed.
'Dir.clickedElement': function(clickEvent) { this.getClickedId(clickEvent) },
'Dir.gotClickedElementId': function(clickedId) { this.prepareClickedModel(clickedId) },
'Dir.clickedModelPrepared': function() { this.model.fetch() },
'Dir.fetchingModel': function() {},
'Dir.doneFetchingModel': function() { this.createModelElement(); },
'Dir.modelReadyToInsert': function(modelElement) { this.insertModelElementInDOM(modelElement) },

// A little spinner
'Dir.spinnerOn': function(spinnerMessage) {
switch(spinnerMessage) {
case 'wait': var spinnerHTML = ''; break;
case 'load':

Interesting code. As you might have guessed, there's a lot wrong with it.

• Events instead of chaining. Makes it harder to follow the code, makes it harder on IDE's that know how to jump to a function, no added benefits. You have this basically as a holder of global variables instead of calling functions with parameters

-
Comments -> I like it, but be careful with things like this:

/***
 * Inserts the clicked model element into the DOM. Fired up by Dir.modelReadyToInsert.
 * @param modelElement
 */
insertModelElementInDOM: function(modelElement) {
    jQuery(this.results_area).html(modelElement);
}

I understand you call this function with the clicked model element, but really you want to document what the function does, not with what it is called.

• this.view = new this.AppView().on({ -> Oh my goodness, because you are abusing events, that parts looks terrible with the lack of newlines, I am not sure you can salvage it without keeping only the real events.

• Regardless, 'Dir.parseForm': function() { this.parseForm() }, could be 'Dir.parseForm': this.parseForm,

• Backbone.View.extend({, the events part, has a ton of copy pasted function() { this.trigger('Dir.parseForm') },, you should do something about that

• You are not using a module loading library -> I think that's ok

• Except for abuse of chaining, I think your code structure is fine

• jQuery -> better than writing and maintaining attr & html yourself I would think

• Creating Models on the fly -> yeah, it looks wrong to me, you are supposed to declare your model once and then get instances.

All in all, I am sorry to say that most of this code should be rewritten to stop abusing events/listeners.

StackExchange Code Review Q#44283, answer score: 5