Refactoring ExtJS for readability

By kenglish

An old colleague once remarked that Perl is “write-only.” That wouldn’t be funny if it wasn’t true. I think JavaScript has a similar property. Being up to my eyeballs in ExtJS lately, I have been trying to come up with ways to make my code a little more manageable. One thing any JavaScript programmer hates is variable scoping bugs. They appear all the time and take 20 hours to fix.

One strategy I’ve been using with ExtJS is use the function Ext.getCmp and local variables instead of global variables. Let’s take another look at the ExtJS basic dataview example to see how this would work.

    var panel = new Ext.Panel({
        id:'images-view',
        frame:true,
        width:535,
        autoHeight:true,
        collapsible:true,
        layout:'fit',
        title:'Simple DataView (0 items selected)',

        items: new Ext.DataView({
            store: store,
            tpl: tpl,
            autoHeight:true,
            multiSelect: true,
            overClass:'x-view-over',
            itemSelector:'div.thumb-wrap',
            emptyText: 'No images to display',

            plugins: [
                new Ext.DataView.DragSelector(),
                new Ext.DataView.LabelEditor({dataIndex: 'name'})
            ],

            prepareData: function(data){
                data.shortName = Ext.util.Format.ellipsis(data.name, 15);
                data.sizeString = Ext.util.Format.fileSize(data.size);
                data.dateString = data.lastmod.format("m/d/Y g:i a");
                return data;
            },

            listeners: {
                selectionchange: {
                        fn: function(dv,nodes){
                                var l = nodes.length;
                                var s = l != 1 ? 's' : '';
                                panel.setTitle('Simple DataView ('+l+' item'+s+' selected)');
                        }
                }
            }
        })
    });

Notice that the panel variable is referenced in the DataView’s selectionchange listener. To me, this is a recipe for disaster because if I rename the panel variable, I’veĀ  broken my listener. I would rewrite this code as the following:

    var panel = new Ext.Panel({
        id:'images-view',
        frame:true,
        width:535,
        autoHeight:true,
        collapsible:true,
        layout:'fit',
        title:'Simple DataView (0 items selected)',

        items: new Ext.DataView({
            store: store,
            tpl: tpl,
            autoHeight:true,
            multiSelect: true,
            overClass:'x-view-over',
            itemSelector:'div.thumb-wrap',
            emptyText: 'No images to display',

            plugins: [
                new Ext.DataView.DragSelector(),
                new Ext.DataView.LabelEditor({dataIndex: 'name'})
            ],

            prepareData: function(data){
                data.shortName = Ext.util.Format.ellipsis(data.name, 15);
                data.sizeString = Ext.util.Format.fileSize(data.size);
                data.dateString = data.lastmod.format("m/d/Y g:i a");
                return data;
            },

            listeners: {
                selectionchange: {
                        fn: function(dv,nodes){
                                var l = nodes.length;
                                var s = l != 1 ? 's' : '';
                                var myPanel = Ext.getCmp('images-view');
                                myPanel.setTitle('Simple DataView ('+l+' item'+s+' selected)');
                        }
                }
            }
        })
    });

I’m using Ext.getCmp to get the panel object. This is a lot more readable because the next person that edits this code knows that object I’m referencing is initialized with id ‘images-view’.

Your comments are welcome.


,

categoriaProgramming commento1 Comment dataDecember 17th, 2008

About... kenglish

This author published 76 posts in this site.

Share

FacebookTwitterEmailWindows LiveTechnoratiDeliciousDiggStumbleponMyspaceLikedin

Comments


Juan Mendes
October 25th, 2010

Hello, I think using Ext.getCmp with and id is a bad pattern. Here’s why: What if you need to use two of these components on a page? you’re out of luck since you’re relying on an id. For the example you mentioned, you should have just set the scope of the selection change to be the current panel by passing ‘this’ as the scope argument. What you have is not that much better than globals, since Ext.getCmp manages a global list of components.

Leave a comment