15

I'm new to extjs and I'm using the MVC architecture.

When my application references a method of a controller, I do it that way (in MyApp.Application):

Mb.app.getController('Main').myMethod();

It is already long, but I think this is the way to do.

When a controller calls it's own method in a closure, I was led to use this code (in MyApp.controller.Main:

controllerMethodOne: function(){
    Ext.Ajax.request({
        url: ...,
        params: ...,
        success: (function(response){
            list = Ext.JSON.decode(response.responseText);
            list.forEach(function(item){
                storeMenu.add(
                    Ext.create('Ext.menu.Item', {
                        text: item.text,
                        handler: function(el){MyApp.app.getController('Main').controllerMethodTwo()}
                    })
                )
            })
        })
    })
},

I referenced the method with MyApp.app.getController('Main').controllerMethodTwo() because this is not refering to the controller object in the closure, and thus this..controllerMethodTwo()isn't working.

I find this utterly convoluted, and I hope someone has an idea to get around that MyApp.app.getController-workaround.

Update

Thanks to all the suggestion I could optimize my code and came up with:

// in my controller
    mixins: ['Mb.controller.mixin.StoreMenu'],
    // I use that style of menus in two controllers thats why I use a mixin
    init: function() {
        this.control({
            '#vg_storeMenu menuitem': {
                click: this.onStoreMenuClicked
            }
        })
    },

// the controller mixin
Ext.define('Mb.controller.mixin.StoreMenu', {
    extend: 'Ext.app.Controller',
    buildStoreMenu: function(store_name){
        var storeMenu = Ext.ComponentQuery.query('#' + store_name + 'Menu')[0];
        Ext.Ajax.request({
            url: Paths.ajax + 'json.php',
            params: {list: store_name + 's'},
            success: (function(response){
            list = Ext.JSON.decode(response.responseText);
            items = Ext.Array.map(list, function(item) {
                return {
                    xtype: 'menuitem',
                    text: item.text
                }
            });
                storeMenu.add(items);
            })
        })
    },
    onStoreMenuClicked: function(el){
        ...
    }
});
Community
  • 1
  • 1
Lorenz Meyer
  • 17,418
  • 21
  • 68
  • 109

2 Answers2

33

Actually, there are at least four distinctly different problems in your code:

  • Scope handling for intra-class method calls
  • Component creation inefficiency
  • Component event handling in a controller
  • Inter-controller communication

Scope handling

The first one is solved either by using a closure, or passing in the scope parameter to Ajax request, as @kevhender described above. Given that, I'd advocate writing clearer code:

controllerMethodOne: function() {
    Ext.Ajax.request({
        url: ...,
        params: ...,
        scope: this,
        success: this.onMethodOneSuccess,
        failure: this.onMethodOneFailure
    });
},

// `this` scope is the controller here
onMethodOneSuccess: function(response) {
    ...
},

// Same scope here, the controller itself
onMethodOneFailure: function(response) {
    ...
}

Component creation

The way you create menu items is less than efficient, because every menu item will be created and rendered to the DOM one by one. This is hardly necessary, either: you have the list of items upfront and you're in control, so let's keep the code nice and declarative, as well as create all the menu items in one go:

// I'd advocate being a bit defensive here and not trust the input
// Also, I don't see the `list` var declaration in your code,
// do you really want to make it a global?
var list, items;

list  = Ext.JSON.decode(response.responseText);
items = Ext.Array.map(list, function(item) {
    return {
        xtype: 'menuitem',
        text: item.text
    }
});

// Another global? Take a look at the refs section in Controllers doc
storeMenu.add(items);

What changes here is that we're iterating over the list and creating a new array of the soon-to-be menu item declarations. Then we add them all in one go, saving a lot of resources on re-rendering and re-laying out your storeMenu.

Component even handling

It is completely unnecessary, as well as inefficient, to set a handler function on every menu item, when all this function does is call the controller. When a menu item is clicked, it fires a click event - all you need to do is to wire up your controller to listen to these events:

// Suppose that your storeMenu was created like this
storeMenu = new Ext.menu.Menu({
    itemId: 'storeMenu',
    ...
});

// Controller's init() method will provide the wiring
Ext.define('MyController', {
    extend: 'Ext.app.Controller',

    init: function() {
        this.control({
            // This ComponentQuery selector will match menu items
            // that descend (belong) to a component with itemId 'storeMenu'
            '#storeMenu menuitem': {
                click: this.controllerMethodTwo
            }
        });
    },

    // The scope is automatically set to the controller itself
    controllerMethodTwo: function(item) {
        ...
    }
});

One best practice is to write the ComponentQuery selectors as finely grained as feasible, because they're global and if you're not precise enough your controller method may catch events from unwanted components.

Inter-controller communication

This is probably a bit far fetched at the moment, but since you're using Ext JS 4.2 you may as well take advantage of the improvements we've added in that regard. Before 4.2, there was a preferred (and only) approach to call one controller's methods from another controller:

Ext.define('My.controller.Foo', {
    extend: 'Ext.app.Controller',

    methodFoo: function() {
        // Need to call controller Bar here, what do we do?
        this.getController('Bar').methodBar();
    }
});

Ext.define('My.controller.Bar', {
    extend: 'Ext.app.Controller',

    methodBar: function() {
        // This method is called directly by Foo
    }
});

In Ext JS 4.2, we've added the concept of event domains. What it means is that now controllers can listen not only to component's events but to other entities events, too. Including their own controller domain:

Ext.define('My.controller.Foo', {
    extend: 'Ext.app.Controller',

    methodFoo: function() {
        // Effectively the same thing as above,
        // but no direct method calling now
        this.fireEvent('controllerBarMethodBar');
    }
});

Ext.define('My.controller.Bar', {
    extend: 'Ext.app.Controller',

    // Need some wiring
    init: function() {
        this.listen({
            controller: {
                '*': {
                    controllerBarMethodBar: this.methodBar
                }
            }
        });
    },

    methodBar: function() {
        // This method is called *indirectly*
    }
});

This may look like a more convoluted way to do things, but in fact it's a lot simpler to use in large(ish) apps, and it solves the main problem we've had: there is no need for hard binding between controllers anymore, and you can test each and every controller in isolation from others.

See more in my blog post: Controller events in Ext JS 4.2

Alex Tokarev
  • 4,718
  • 1
  • 18
  • 30
  • 1
    Thanks alot ! Your answer exceed by far my expectations. – Lorenz Meyer Oct 08 '13 at 18:48
  • Perhaps another option to add is application level events. Have controller fire event from the application object an listen to them from another controller as proposed in Tommy's MVC guide part2 http://docs.sencha.com/extjs/4.1.3/#!/guide/mvc_pt2-section-setting-up-listeners – dbrin Oct 08 '13 at 21:09
  • @dbrin Application level (or global) events has been available since Ext JS 4.0 in a hackish way, and since 4.1 in more officially supported way; however that approach shares the same deficiency as direct controller method calling: you have a hard binding, this time on the application itself. Actually it's even worse because in order to test your code, you'll have to either load the whole application, or stub it out intensively. Using controller event domain solves all these problems without introducing negative side effects, and is the recommended way to do things in 4.2+. – Alex Tokarev Oct 08 '13 at 21:14
  • Good to know, thanks. As far as testing is concerned, I would love to see a sane approach to pure unit testing with Jasmine (DOMless). I have seen a bunch of attempts out there but not anything I would consider gold standard. Fiesta is great, but more of a full functional testing approach. – dbrin Oct 08 '13 at 22:11
  • Hi all, unfortunately in extjs 6.3.5 getController in Ext.app.ViewController is not a valid method, how can I call another controller from inside a ViewController? – Stefano Scarpanti Dec 18 '18 at 09:52
2

this doesn't work in the success callback because it doesn't have the right scope. Your 2 options are to:

1: Create a variable at the beginning of the function to reference in the callback:

controllerMethodOne: function(){
    var me = this;
    Ext.Ajax.request({
        url: ...,
        params: ...,
        success: (function(response){
            list = Ext.JSON.decode(response.responseText);
            list.forEach(function(item){
                storeMenu.add(
                    Ext.create('Ext.menu.Item', {
                        text: item.text,
                        handler: function(el){me.controllerMethodTwo()}
                    })
                )
            })
        })
    })
},

2: Use the scope config of the Ext.Ajax.request call:

controllerMethodOne: function(){
    Ext.Ajax.request({
        url: ...,
        params: ...,
        scope: this,
        success: (function(response){
            list = Ext.JSON.decode(response.responseText);
            list.forEach(function(item){
                storeMenu.add(
                    Ext.create('Ext.menu.Item', {
                        text: item.text,
                        handler: function(el){me.controllerMethodTwo()}
                    })
                )
            })
        })
    })
},
kevhender
  • 4,246
  • 1
  • 11
  • 16
  • The second solution looks especially nice. I'll give it a try tomorrow. – Lorenz Meyer Oct 08 '13 at 18:07
  • I generally prefer the first, to help reduce confusion over use of `this`, and it can save characters when minifying... but it is really a matter of personal preference. – kevhender Oct 08 '13 at 19:11