Refactoring suggestions

Subscribe to Refactoring suggestions 1 post, 1 voice

 
Avatar kmandrup 1 post

I have just been looking into the source of GamingTrends, since I want to use Rails with ExtJS and find it a somewhat good inspiration.

To get up and running I first had to setup the MySql database with the schema. I had problems using the migrations. I got some errors on the E304 and descrip index og Games. (BLOG/TEXT requires a length key, tried adding a :limit => 1000, but didn’t work!?)

To import the schema into a mysql database, I recommend removing Games indexes “E304” and “descrip” from the first migration. They are deleted on the cleanup migrations anyways!

Finally I got the app up and running, but I was unable to create Posts!

in the source code
/app/views/post/index.rhtml
- Contains Headline x 2, missing FrontPage attribute, title should be “Create New Story”

/app/views/post/new.rhtml
- Same errors as in index.rhtml, replace second (duplicate) “Headline” with “FrontPage”

Looking into the code:

I feel the ExtJS infrastructure can be vastly improved by scrapping the ext helper function in application_helper.rb and refactoring them.
The current design is not very DRY and the multiple ” function << ...” statements to build up the ExtJS javascript is not exactly pretty.

I came up with the following refactoring solution:
--------

application_helper.rb
--— # model, primary_key, fields, columns, height, width, pageSize, newsFun def ext_grid(options = {}) xml_fields = fields xml_fields << primary_key xml_fields.collect! {|x| ”’#{x}’”} models = model.pluralize options.merge!(xml_fields, models) render_ext_js(‘ext_grid’, options) render_ext_js(‘news_fun’, options) if newsFun end

def render_ext_js(name, options)
  javascript_tag(render(:partial => "shared/ext_grid", :locals => options))
end

games/index.rhtml
---

<= ext_layout(‘Games’) %>
<
= ext_grid(:optons => {:model => “game”, :primary_key => “GameID”, :fields => %w(Title Console), :columns => “{header: ‘Title’, width: 300, dataIndex: ‘Title’}, {header: ‘Console’, width: 250, dataIndex: ‘Console’}”, :height => 340, :width => 530, :pageSize => 13}) %>

shared/_ext_grid.rhtml
----
var pageGrid = function() { var grid; var dialog; var ds;

return { 
    init : function(){
         ds = new Ext.data.Store({    
         proxy: new Ext.data.HttpProxy({
            method: 'GET', 
            url: '<%= models %>.json'}),
            reader: new Ext.data.JsonReader({
                root: '<%= models %>',
                 totalProperty: 'totalCount',
                 id: '<%= primary_key %>'
            }, 
}
}();
});    
var cm = new Ext.grid.ColumnModel([<%= columns %>]);
cm.defaultSortable = true;
grid = new Ext.grid.Grid(
    'content', 
    { ds: ds, cm: cm }
);    
grid.render();
var gridFoot = grid.getView().getFooterPanel(true);
var paging = new Ext.PagingToolbar(gridFoot, ds, {
    pageSize: <%= pageSize %>,
    displayInfo: true,
    displayMsg: 'Displaying topics {0} - {1} of {2}',    
    emptyMsg: 'No topics to display'
});
ds.load({
    params:{start:0, limit:<%= pageSize %>}
});
grid.on('rowdblclick', editResource);
var gridHead = grid.getView().getHeaderPanel(true);
var tb = new Ext.Toolbar(gridHead);
tb.add(
    { 
        text: 'Create New <%= model %>', 
        handler: createResource 
    }, 
    '-', 
    { 
        text: 'Delete Selected  <%= model %>', 
        handler: deleteResource 
    }
);
tb.add(
    '-', 
    'Filter: ', 
    "<input type="text" id="text_filter">" 
);
Ext.get('text_filter').on('keyup', filterResource);
}

PageGrid.filterResource = function() { filtervalue = Ext.get(‘text_filter’).dom.value; ds.proxy = new Ext.data.HttpProxy({ method: ‘GET’, url: ‘<%= models %>.json?search=’ + filtervalue} ); ds.reload();
}

PageGrid.deleteResource = function() { var id = grid.getSelectionModel().getSelected(); if(id){ Ext.MessageBox.confirm(‘Confirm’, ‘Are you sure you want to delete this <%= model %>?’, postDelete); } else { Ext.MessageBox.alert(‘DOH!’, ‘Maybe you want to try again after ACTUALLY selecting something?’) }
}

PageGrid.submitResource = function(){ document.create_resource.submit();
}

PageGrid.postDelete = function(btn){ if(btn == ‘yes’) { var id = grid.getSelectionModel().getSelected(); var deleteme = id.get(‘<%= primary_key %>‘); window.location.href = ’/<%= models %>/destroy/’ + deleteme; }
}

PageGrid.editResource = function (grid, rowIndex) { var id = grid.getSelectionModel().getSelected(); if(id) { window.location.href = ’/<= models %>/’ + id.get(‘<= primary_key %>‘); }
}
// width:500, 630, pageSize:13, 20
PageGrid.createResource = function() { if(!dialog) { dialog = new Ext.BasicDialog(‘newDialog’, { width: <%= width %>, height:<%= height %>, shadow:true, minWidth:300, minHeight:<%= height %>, proxyDrag:true, autoScroll:false, animEl:true }); dialog.addKeyListener(27, dialog.hide, dialog); postBtn = dialog.addButton(‘Submit’, submitResource, this); dialog.addButton(‘Close’, dialog.hide, dialog); } dialog.show(); dialog.on(‘hide’, function(){ document.create_resource.reset(); })
}

Ext.onReady(pageGrid.init, pageGrid, true);

shared/_news_fun.rhtml
---

PageGrid.renderNews = function(value, p, record) { return String.format(‘{0}
{1}’, value, Ext.util.Format.stripTags(record.data[‘Body’]).substr(0,100) + ”....”);
}

PageGrid.renderNewsPlain = function(value) { return String.format(‘{0}’, value);
}

PageGrid.toggleNews = function(btn, pressed) { cm.getColumnById(‘Headline’).renderer = pressed ? renderNews : renderNewsPlain; grid.getView().refresh();
}

var formatBoolean = function (value) { return (value == 1) ? ‘Yes’ : ‘IMO. I will see where I get from here…