marionettejs/backbone.marionette — pull request #263 by derickbailey (2012-09-29 17:45:47+00:00)
Title: When closing a view instance multiple times, only run the closing code once

This is a merger of pull request #257 and some additional changes in behavior and specifications to show that closing a view multiple times only closes the view once.

I'm using this PR as a way of facilitating code review for two things:

1) to make sure this the behavior that we want

2) to see if the current implementation is sufficient, as it does add a little bit of complexity to the close method and overriding it.

In order to preserve the behavior of only running the `close` code once when calling `close` multiple times, I had to set up the base view's close method to accept a callback function. The base close method checks to see if the view is already closed. If it is, it exits immediately. If not, it calls the callback function. The callback function is then handed a callback of it's own - namely the actual `close` function that performs all of the work to close the base view. This allows any sub-type to override the close view and add pre and post closing logic and code, while still facilitating the behavior of "only run the close code once".

The problem is, if someone wishes to extend from any of Marionette's views and override the close method, there is now an additional burden on the developer to properly set up the close function call. I've illustrated this in the View documentation, and am copying here:

---
When overriding in sub-types, the `cb` parameter is a callback
that receives the `close` inner method. The `View.prototype.close`
method should be called like this:

```js
Backbone.Marionette.ItemView.extend({

// ...

close: function(cb){
// any code here will always be run when the view's `close` method is called

Backbone.Marionette.View.prototype.close.call(this, function(close){
// code in here will only be run if the view is not already closed

// run any pre-close code here

// close it, allowing other views to extend from this one with the
// same pattern of a callback function
if (cb){
cb.call(this, close);
} else {
close();
}

// run any post-close code here
});

}
});
```
---

The question, then, is whether or not this set up is acceptable. Will this be too confusing and too much work for those that are extending from Marionette's views? Is there a way that this can be simplified and still preserver the desired behavior?

Any and all feedback is greatly appreciated.