7
\$\begingroup\$

I have a little js I call a "jqGrid Factory" to encapsulate common settings and functionality across my web app.

I just want to see what improvements I can make.

 var jqGridReportFactory = (function () {

    var config = {
            datatype: 'json',
            mtype: 'GET',
            height: 'auto',
            autowidth: true,
            shrinkToFit: true,
            gridview: true,
            sortable: true,
            rowNum: 50,
            rowList: [50, 100, 200],
            viewrecords: true,
            loadonce: false,
            sortorder: 'asc',
            sortname: 'Affiliate',
            subGridSortname: 'SubAffiliate'
        },
        subGridOptions = {
            plusicon: "ui-icon-plus",
            minusicon: "ui-icon-minus",
            openicon: "ui-icon-carat-1-sw"
        };

    function createReport(gridOptions, optionalConfig) {
        $.extend(config, optionalConfig);
        //$.extend(gridOptions, gridOptions);

        var jqGridObj = {
            url: gridOptions.url,
            datatype: config.datatype,
            mtype: config.mtype,
            postData: gridOptions.postData,
            colNames: gridOptions.colNames,
            colModel: gridOptions.colModel,
            height: config.height,
            autowidth: config.autowidth,
            shrinkToFit: config.shrinkToFit,
            gridview: config.gridview,
            sortable: config.sortable,
            rowNum: config.rownum,
            rowList: config.computeHighlightColorsrowList,
            viewrecords: config.viewrecords,
            loadonce: config.loadonce,
            sortorder: config.sortorder,
            sortname: gridOptions.sortname,
            pager: gridOptions.pager,
            loadError: function (xhr, st, err) {
                reportLoadError('onLoadConversionHistory', xhr, st, err);
                unblockUI();
            },
            gridComplete: function () {
                unblockUI();
                goToScrollPosition($('#reportPlaceHolder'));
            },
            subGrid: gridOptions.subGrid,
            onSelectRow: function (rowid) {
                $(this).jqGrid("toggleSubGridRow", rowid);
            }
        };

        if (gridOptions.subGrid) {
            jqGridObj = addSubGrid(jqGridObj, gridOptions);
        }

        //jqGrid factory go!!
        $("#" + gridOptions.id).jqGrid(jqGridObj);
    }

    function addSubGrid(jqGridObj, gridOptions) {
        var subGridObj = {
            subGridOptions: subGridOptions,
            subGridRowExpanded: function (subgridId, rowId) {
                var affiliate = $("#" + id).jqGrid("getCell", rowId, 'Affiliate');

                var subgridTableId = subgridId + "_t";
                var $subGrid;
                $("#" + subgridId).html("<table id='" + subgridTableId + "' class='scroll'></table>");
                $subGrid = $('#' + subgridTableId); //cache subgrid, more performant

                var subGridColNames = jQuery.extend({}, gridOptions.colNames);
                var subGridColModel = jQuery.extend({}, gridOptions.colModel);
                //change parent names from Affiliate to Subaffiliate
                //other than that subGrid model is exactly the same as parent Affiliate model for all reports so far

                subGridColNames[0] = 'SubAffiliate';
                subGridColModel[0].name = 'SubAffiliate';
                subGridColModel[0].index = 'SubAffiliate';

                //add affiliate to subGridPostData
                var a = { Affiliate: affiliate };
                $.extend(gridOptions.subGridPostdata, a);

                $subGrid.jqGrid({
                    url: gridOptions.url,
                    datatype: gridOptions.datatype,
                    mtype: gridOptions.mtype,
                    postData: gridOptions.subGridPostdata,
                    colNames: subGridColNames,
                    colModel: subGridColModel,
                    height: config.height,
                    sortname: config.subGridSortname,
                    sortorder: config.sortorder,
                    loadonce: config.loadonce,
                    //these subgrid setting should not be overridden in my opinion - Brian Ogden
                    autowidth: true,
                    shrinkToFit: true,
                    gridview: false,
                    sortable: false,
                    viewrecords: true
                    ///////////////////////
                });

                if (gridOptions.subGridHeadersHidden) {
                    //hide subgrid column headers
                    $subGrid.closest("div.ui-jqgrid-view")
                    .children("div.ui-jqgrid-hdiv")
                    .hide();
                }
            },
            subGridRowColapsed: function (subgridId, rowId) {
                // this function is called before removing the data 
                var subgridTableId;
                subgridTableId = subgridId + "_t"; //
                $("#" + subgridTableId).remove();
            }
        };

        $.extend(true,jqGridObj, subGridObj);

        return jqGridObj;
    }

    return {
        createReport: createReport
    };

})();

UPDATE

Just wanted to show the latest of my refactoring of my jqgrid factory, I didn't renamed some of the options object members to camel case because I thought it was better for the users of this factory to match the option value that jqGrid uses:

var jqGridReportFactory = (function () {

    var constAffiliateStr = "Affiliate";
    var constSubAffiliateStr = "SubAffiliate";

    //default icons should be used for all reports so they not currently an option that can changed when using this jqgrid factory - Brian Ogden  1-24-2014
    var subo = {
        plusicon: "ui-icon-plus",
        minusicon: "ui-icon-minus",
        openicon: "ui-icon-carat-1-sw"
    };

    function createReport(o) {

        o = $.extend({
            //apply default properties
            datatype: 'json',
            mtype: 'GET',
            height: 'auto',
            autowidth: true,
            shrinkToFit: true,
            gridview: true,
            sortable: true,
            rowNum: -1,
            rowList: [50, 100, 200, -1],
            viewrecords: true,
            loadonce: true,
            footerrow: false,
            sortorder: 'asc',
            sortname: constAffiliateStr,
            subGridSortname: constSubAffiliateStr,
            subgrid: false,
            subGridHeadersHidden: true
        }, o);

        var jqGridObj = {
            url: o.url,
            datatype: o.datatype,
            mtype: o.mtype,
            postData: o.postData,
            colNames: o.colNames,
            colModel: o.colModel,
            height: o.height,
            autowidth: o.autowidth,
            shrinkToFit: o.shrinkToFit,
            gridview: o.gridview,
            sortable: o.sortable,
            rowNum: o.rowNum,
            rowList: o.rowList,
            viewrecords: o.viewrecords,
            loadonce: o.loadonce,
            sortorder: o.sortorder,
            sortname: o.sortname,
            pager: o.pager,
            footerrow: true,
            loadError: function (xhr, st, err) {
                reportLoadError('onLoad' + o.id, xhr, st, err);
                unblockUI();
            },
            loadComplete: function () {
                if (o.rowNum == -1) {
                    $(o.pager + ' option[value=-1]').text('All');
                    //if loadOnce is true displays -1 
                    if (o.loadonce) $(o.pager + ' input.ui-pg-input').next().text('1');
                }

                if (o.loadComplete) o.loadComplete();
            },
            gridComplete: function () {
                unblockUI();
                goToScrollPosition($('#reportPlaceHolder'));

                if (o.gridComplete) o.gridComplete();

            },
            subGrid: o.subGrid,
            onSelectRow: function (rowid) {
                $(this).jqGrid("toggleSubGridRow", rowid);
            },
            onSortCol: function (index, idxcol, sortorder) {
                var $icons = $(this.grid.headers[idxcol].el).find(">div.ui-jqgrid-sortable>span.s-ico");
                if (this.p.sortorder === 'asc') {
                    //$icons.find('>span.ui-icon-asc').show();
                    $icons.find('>span.ui-icon-asc')[0].style.display = "";
                    $icons.find('>span.ui-icon-asc')[0].style.marginTop = "0px";
                    $icons.find('>span.ui-icon-desc').hide();
                } else {
                    //$icons.find('>span.ui-icon-desc').show();
                    $icons.find('>span.ui-icon-desc')[0].style.display = "";
                    $icons.find('>span.ui-icon-desc')[0].style.marginTop = "0px";
                    $icons.find('>span.ui-icon-asc').hide();
                }
            }
        };

        /*=================================================*/
        //Build subGrid
        /*=================================================*/
        if (o.subGrid) {

            /*=================================================*/
            //Check to see if subGrid colModel and colNames passed in, if not use Affiliate colModel and colNames
            /*=================================================*/
            var temp;
            if (!o.subGridColModel) {
                temp = $.extend(true, [], o.colModel); //deep copy array of objects
                temp[0].name = constSubAffiliateStr;
                temp[0].index = constSubAffiliateStr;
                o.subGridColModel = temp;
            }

            if (!o.subGridColNames) {
                //temp = o.colNames.slice(0);
                temp = $.extend(true, [], o.colNames); //deep copy not needed but better safe then sorry
                temp[0] = constSubAffiliateStr;
                o.subGridColNames = temp;
            }
            /*=================================================*/


            var subGridObj = {
                subo: subo,
                subGridRowExpanded: function (subgridId, rowId) {

                    var affiliate = $("#" + o.id).jqGrid("getCell", rowId, 'Affiliate');

                    var subgridTableId = subgridId + "_t";
                    var $subGrid;
                    $("#" + subgridId).html("<table id='" + subgridTableId + "' class='scroll'></table>");
                    $subGrid = $('#' + subgridTableId); //cache subgrid, more performant

                    //add affiliate to subGridPostData
                    var a = { Affiliate: affiliate };
                    $.extend(o.subGridPostData, a);

                    $subGrid.jqGrid({
                        url: o.url,
                        datatype: o.datatype,
                        mtype: o.mtype,
                        postData: o.subGridPostData,
                        colNames: o.subGridColNames,
                        colModel: o.subGridColModel,
                        height: o.height,
                        sortname: o.subGridSortname,
                        sortorder: o.sortorder,
                        loadonce: o.loadonce,
                        sortable: o.sortable,
                        //these subgrid setting should not be overridden in my opinion - Brian Ogden
                        autowidth: true,
                        shrinkToFit: true,
                        gridview: false,
                        viewrecords: true
                        ///////////////////////
                    });

                    if (o.subGridGroupHeaders) {
                        $subGrid.jqGrid('setGroupHeaders', {
                            useColSpanStyle: true,
                            groupHeaders: o.subGridGroupHeaders
                        });
                    }

                    if (o.subGridHeadersHidden) {
                        //hide subgrid column headers
                        $subGrid.closest("div.ui-jqgrid-view")
                        .children("div.ui-jqgrid-hdiv")
                        .hide();
                    }
                },
                subGridRowColapsed: function (subgridId, rowId) {
                    // this function is called before removing the data 
                    var subgridTableId;
                    subgridTableId = subgridId + "_t"; //
                    $("#" + subgridTableId).remove();
                }
            };

            $.extend(jqGridObj, subGridObj);
        }
        /*=================================================*/

        //jqGrid factory go!!
        return $("#" + o.id).jqGrid(jqGridObj);
    }

    return {
        createReport: createReport
    };

})();
\$\endgroup\$
4
\$\begingroup\$

I like your code, good use of the Revealing Module pattern.

I only have the following minor observations:

  • mtype could use a better name ( I know, jqGrid uses it )
  • autowidth -> autoWidth ( lowerCamelCasing )
  • gridview -> gridView
  • rowList: [50, 100, 200] could use a comment as to what it does
  • viewrecords -> viewRecords( lowerCamelCasing ) etc. etc.
  • sortorder -> Could use a comment with the possible values
  • subGridOptions -> could use a comment with this URL : http://api.jqueryui.com/theming/icons/
  • delete uncommented code : //$.extend(gridOptions, gridOptions);
  • You should seriously consider building jqGridObj with $.extend out of gridOptions and config, cutting almost 20 lines.
  • It is considered better to have 1 var statement with comma-separated variables instead of multiple var statements. ( in subGridObj )
  • $subGrid.jqGrid could also probably be built with $.extend
\$\endgroup\$
  • \$\begingroup\$ Hi @konijn,thanks for your review,I was wondering,could you elaborate with a snippet of how I might build $subGrid.jqGrid with $.extend? \$\endgroup\$ – Brian Ogden Jan 23 '14 at 23:40
  • \$\begingroup\$ Oh nevermind! I understand what you mean by building $subgrid.jqGrid with $.extend now, instead of calling the function addSubGrid to build it I believe is what you mean \$\endgroup\$ – Brian Ogden Jan 23 '14 at 23:48
  • 1
    \$\begingroup\$ That's indeed what I meant, I guess you would extract your subgrid defaults into a properly named variable first. \$\endgroup\$ – konijn Jan 23 '14 at 23:49
  • \$\begingroup\$ I have made some improvements if you want to check them out. \$\endgroup\$ – Brian Ogden Jan 25 '14 at 1:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.