6
\$\begingroup\$

There's this Ajax-heavy, jQuery-based site which I'm about to work on. There hasn't been much structure in the JavaScript code so far (lots of functions at global scope, hard-coded in the HTML onclick="something(args);". How can I improve upon that and introduce some structure?

Here's what I've tried. This part is supposed to handle a single element which consists of a text field (span) which is replaced by an input on focus and runs an AJAX update call when a submit button is clicked or enter is pressed.

$(function() {
    var box = $('.foobar-info'),
        input = box.find('input'),
        label = box.find('span#name'),
        button = box.find('button');

    label.click(start_edit);
    button.click(submit);
    input.keypress2(KEYS.enter, submit);

    function start_edit() {
        box.addClass('is-edit');
    }
    function submit() {
        MyAjaxModule.edit_foobar(save_success,
            {'id': GLOBAL_FOO_ID, 'value': input.val()});
    }
    function save_success(data) {
        label.text(input.val());
        box.removeClass('is-edit');
    }
});

Showing and hiding of parts is driven by the CSS:

.foobar-info .state-display {display: inline;}
.foobar-info .state-edit {display: none;}
.foobar-info.is-edit .state-display {display: none;}
.foobar-info.is-edit .state-edit {display: inline;}

So far this achieves:

  • keeps the behaviour away from the markup
  • keeps relevant behaviour together in one block

On the other hand, I don't think it's testable or particularly easy to re-use with different configurations.

I've attempted to rewrite this to use objects and this. lookups instead of the big closure, but didn't end up in a code that would please me. For example, this part

label.click(start_edit);

would look like

this.label.click(this.start_edit.bind(this));

which looks really verbose plus it's super-easy to forget the unwieldy .bind(this) part.

This article served some inspiration, but felt rather hacky to me, mostly because relying on global name lookups.

Questions:

  • Any particular problems with my approach except what I've mentioned so far? How can I solve it?
  • Is there a canonical way to structure the code which I'll be happy to incorporate?
\$\endgroup\$
  • \$\begingroup\$ The Module Pattern mentioned in that article is really great for projects like this because you can add/remove functionality in a jiffy. Also check out the Observer Pattern, aka Pub/Sub, which might be helpful with structuring your AJAX calls. \$\endgroup\$ – Jonny Sooter Jul 2 '13 at 18:52
2
\$\begingroup\$

There are two schools of thought

  • Programmatic
  • Declarative

Your example here is programmatic, where JavaScript is binding events after the DOM load. The problem I have with programmatic is, using your words, although it "keeps the behavior away from the markup" it moves the markup into the behavior. The interface that is needed to reuse the same JavaScript is now tied to what is defined on the page. Using declarative, your example being onclick="something(args);", I can more easily follow a Model View Presenter (MVP) pattern where the view simply forwards all events to the Presenter to perform the business logic.

Here the events can be organized into a class object to remove them from the global namespace. Scope can be controlled by passing in the model as a parameter.

presenter.js

function Presenter(parameter) {

  // private variables
  var myVar;

  // public method
  this.submit = function() {
  };

  // private method
  function doSomething() {
  }
}

page.html

...
<script type="text/javascript" src=".../presenter.js"></script>
<script type="text/javascript">
  var presenter = new Presenter(...);
</script>
...
<button onclick="presenter.onButtonClick();">Click me</button>
\$\endgroup\$
1
\$\begingroup\$

If this is a sizable component on your page which has state and a UI that reflects that state and, optionally, the data is server-backed, you might want to invest in the use of a framework. Try MVC (or MVC-like) frameworks like Backbone.js and others. In addition, use templating frameworks for the UI. My choice is Mustache, but still, there are a lot more.

This will split your code into the View, which is your UI. There's the Controller, otherwise known as the logic of the component. Then there's the Model, which is the data, usually split between local and server data, synced. Note that some frameworks are not strictly MVC but should serve the same purpose.

By now you might think "Sure, the code is now split up into separate components. But there are many components, what should I do?". Module management comes into play with dependency management libraries. I usually use RequireJS. These libraries manage the proper loading of required scripts before the scripts that require them execute.

So in summary, you split your code into parts of the MVC, glue them together with the dependency manager.


On the other hand, if this is component is somewhat small, then you might opt to make a widget/plugin. You might still need the MVC approach, but instead of using a baked framework, you roll your own. It's as simple as abstracting a few low-level operations into generic functions and data structures and you are good to go.

OOP concepts in JS would be your best friend here, as you can encapsulate several entities into objects. For example, you can create a View object and have it hold the UI elements of interest and several render methods. You can make a Model object which holds a simple data structure, and a few CRUD methods.

\$\endgroup\$
1
\$\begingroup\$

I don't know what those first two answers are about, they're completely inappropriate IMO; There's no way this solution calls for MVC or for declarative syntax. Your code is beautiful exactly the way it is; honestly it's very well done.

I did have a little trouble matching up the variables with the click handlers because you coded them in a different order and I got confused; inlining the calls like this eliminates that problem, and you don't need those temporary variables any more.

$(function() {
    var box = $('.foobar-info'),
    box.find('input').keypress2(KEYS.enter, submit);
    box.find('span#name').click(start_edit);
    box.find('button').click(submit);;
    ///etc.

Naturally you still need the "label" variable so your handler can get it. Although personally I have a thing about keeping my variables very tightly scoped (especially in Javascript where a C# developer tends to be surprised by variable hoisting and other lovely subtleties in Javascript) so I would actually write it all out in the handler, like this:

function save_success(data) {
    $('.foobar-info span#name').removeClass('is-edit');
}

Not saying this is better-- it will have (very) slightly worse performance-- but I think it is clearer and more maintainable. Totally a matter of taste. (If you are going to keep a variable for "label," I would suggest using a more descriptive name. If you're minifying your JS files there's no reason not to use longer names).

One other small note. This selector

$('span#name')

will not perform as well as

$('#name')

and for your purposes should do exactly the same thing, assuming there is only one element with an ID of "name." The subtle difference is the 'span#name' will use jquery's Javascript code and can possibly return a set of elements; using '#name' only will use the getElementById built-in method (which is way faster) and will never return more than one element (usually the first one that matches). In theory a valid DOM document has unique IDs so the distinction is only useful in special situations (e.g. rows generated dynamically that all have the same ID).

While we're going over JQuery internals here's another tip. This line

var box = $('.foobar-info')

is not very efficient on older browsers-- it will result in a complete DOM scan. If possible, prefix with something like

var box = $('div.foobar-info')
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.