Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

restoring controller as instance rather than singleton #676

Merged
merged 1 commit into from

2 participants

@caridy
Owner
  • instance.getController is not instance.createController
  • this change might help to avoid memory leaks
@drewfish
Owner

I don't think we need to change the name of the function. In my mind getController() works well for both cases (create or return singleton) -- it's just a factory.

Later we can add a configuration option so that config in application.json (or perhaps even in the mojit's definition.json) can decide whether a new controller instance is created or not.

@drewfish
Owner

Even without the changes mentioned above, +1.

@caridy
Owner

Yeah, I decided to change it because:

  • getController or createController are only relevant for testing because the instance object is created per request per mojit instance using the blending mechanism. I believe it should be removed.

  • it is not really a factory, because you're going to get a new method on every dispatch.

  • "createController" is more descriptive, and follows our current guideline, like store.createStore() etc.

@caridy caridy merged commit a1a414c into yahoo:develop-perf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 29, 2012
  1. @caridy
This page is out of date. Refresh to see the latest.
View
14 lib/app/autoload/dispatch.common.js
@@ -71,17 +71,21 @@ YUI.add('mojito-dispatcher', function(Y, NAME) {
Y.mojito.perf.mark('mojito', 'core_dispatch_start',
'dispatching an instance', command);
- // Ensure there's a getController method we can call
+ // Ensure there's a createController method we can call
// that will always return a viable controller. By
// wrapping in a function we allow tests and other code
// to provide mocks etc.
- instance.getController = instance.getController ||
+ // TODO: instance.createController should be part of the
+ // expandInstance routine rather than dispatch.
+ instance.createController = instance.createController ||
function() {
- return Y.mojito.controllers[
+ // returning a controller instance instead of
+ // a singleton to avoid leaks.
+ return Y.mojito.util.heir(Y.mojito.controllers[
instance.controller
- ];
+ ]);
};
- controller = instance.getController();
+ controller = instance.createController();
perf = Y.mojito.perf.timeline('mojito', 'ac:ctor',
'create ControllerContext', command);
View
6 tests/unit/lib/app/autoload/test-dispatch.common.js
@@ -33,7 +33,7 @@ YUI.add('mojito-dispatcher-tests', function(Y, NAME) {
id: 'xyz123',
instanceId: 'xyz123',
'controller-module': 'dispatch',
- getController: function() {
+ createController: function() {
return { index: function() {} };
},
yui: {
@@ -83,7 +83,7 @@ YUI.add('mojito-dispatcher-tests', function(Y, NAME) {
id: 'xyz123',
instanceId: 'xyz123',
'controller-module': 'dispatch',
- getController: function() {
+ createController: function() {
getterInvoked = true;
return { index: function() {} };
},
@@ -118,7 +118,7 @@ YUI.add('mojito-dispatcher-tests', function(Y, NAME) {
id: 'xyz123',
instanceId: 'xyz123',
'controller-module': 'dispatch',
- getController: function() {
+ createController: function() {
return { index: function() {
actionInvoked = true;
} };
Something went wrong with that request. Please try again.