Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm refactoring a responsive report builder in JavaScript. Here's what it looks like: report builder UI

This started as a small set of objects that transformed data, rendered the graphs with D3.js, and managed the layout all in the same object. The graphs can be removed by the user, and they resize based on screen width and the layout adapts to smaller screen sizes. I've decided to convert each type of graph to a subview and use a parent view to manage the layout of these subviews. This allows me to centralize the code that inserts the graphs into the DOM based on width, platform, etc.

The subviews are all built from a single, large set of homogenous data. I have a single object that constructs these views based on available data then stores the subviews in a collection object (to be persisted when the report is saved). This collection of subviews is then passed to the layout manager for rendering. Subviews are only constructed if the data is available. The thought here is separating object graph construction from the rest of the business logic will make it easier to write unit tests – a requirement moving forward.

I'm having trouble finding a good way to place the subviews on the page. Currently, they're all in a single collection, but they need to be placed in different places based on what kind of graph it is and which set of data the graph is for. e.g. for each of 1-3 "properties", a graph of growth for the month needs to go at the top of the first page and middle of third, while a single graph of top referrers needs to be drawn only on the last page. The most obvious solution would be to simply iterate over all of the subviews and using a giant switch statement, but I think "giant switch statement" and then immediately think "refactor" and "use polymorphism".

One of the ideas I had were to index the subview collection and create a map to lookup the DOM insertion code based on the index, but this is only a stone throw from a giant switch statement, and I'm essentially coupling my layout manager to the subview builder. Another idea was to check the subview instance name in the layout manager, then insert the graph into the appropriate place in the DOM based on this value; however, this would still require a big switch statement.

Is there a good solution to this problem, or is my design flawed?

Here's how the page is initialized (in a Twig template):

// At end of the body...

require([
    '/script/views/graphs-builder.js',
    '/script/views/report.js'
],
function(GraphsBuilder, ReportView) {
    "use strict";

    // Bootstrap data
    var properties = {{ properties | json_encode | raw }},
    reports = {{ reports | json_encode | raw }};

    var builder = new GraphsBuilder(properties, reports);
    builder.build(); // Iterates over reports to construct subviews
    // Stores the subviews in builder.graphs

    var report = new ReportView({
        graphs: builder.graphs
    });

    report.render() // Iterate over subviews, using a big ugly switch statement
    // to decide where in the DOM to insert the graph?

    report.$el.appendTo('.container');
});

Here's the beef of GraphBuilder:

define([
    '/script/graphs/common.js',
    '/script/graphs/facebook.js',
    '/script/graphs/twitter.js',
    '/script/graphs/youtube.js',
    '/script/graphs/analytics.js'
],
function(common, facebook, twitter, youtube, analytics) {
    'use strict';

    function GraphBuilder(properties, reports, social_traffic, keyword_traffic) {
        this.properties = properties;
        this.reports = reports;
        this.social_traffic = social_traffic;
        this.keyword_traffic = keyword_traffic;
        this.graphs = new Backbone.ChildViewContainer();
    }

    _.extend(GraphBuilder.prototype, {
        build: function() {
            var data;

            this.sanitizeReports();
            this.buildSocialGraphs('facebook');
            this.buildSocialGraphs('twitter');
            this.buildSocialGraphs('google.youtube');

            data = _.filter(this.reports, {network: 'google.analytics'});
            if (data.length && _.some(data, this.isDaily)) {
                data = _.filter(data, this.isDaily);
                this.graphs.add(analytics.TrafficByDay(data));
            }

            this.graphs.add(common.TotalReach(_.filter(this.reports, this.isDaily), 'Total reach'));

            if (this.social_traffic) {
                this.graphs.add(analytics.SocialTraffic(this.social_traffic));
            }

            if (this.keyword_traffic) {
                this.graphs.add(analytics.KeywordTraffic(this.keyword_traffic));
            }

            return this;
        },

        buildSocialGraphs: function(network) {
            var graphModule = network === 'facebook' ? facebook :
                network === 'twitter' ? twitter :
                    network === 'google.youtube' ? youtube : analytics;

            var opts;

            var data = _.filter(this.reports, {network: network});
            if (_.some(data, this.isDaily)) {
                var daily = _.filter(data, this.isDaily);
                opts = {data: daily};
                this.graphs.add(graphModule.CommunitySizeByDay(opts), network + '-community-size-by-day');
                this.graphs.add(graphModule.ReachVsEngagementByDay(opts), network + '-reach-vs-engagement-by-day');
            }

            if (_.some(data, 'month')) {
                var monthly = _.filter(data, 'month');
                this.graphs.add(graphModule.CommunitySizeByMonth({data: monthly}), network + '-community-size-by-month');

                if (monthly.length >= 2) { // Need at least two months of data
                    opts = {lastMonth: monthly[0], thisMonth: monthly[1]};
                    this.graphs.add(graphModule.CommunityGrowthThisMonth(opts), network + '-community-growth-this-month');
                    this.graphs.add(graphModule.BroadcastingThisMonth(opts), network + '-broadcasting-this-month');
                }
            }
        }
    });
});

And here's the report view:

define([
    '/script/graphs/common.js',
    '/script/graphs/facebook.js',
    '/script/graphs/twitter.js',
    '/script/graphs/youtube.js',
    '/script/graphs/analytics.js'
], function(common, facebook, twitter, youtube, analytics) {
    'use strict';

    return Backbone.View.extend({
        initialize: function() {
            this.graphs = this.options.graphs;
        },

        defaults: {
            edit_mode: false,
            graphs: new Backbone.ChildViewContainer()
        },

        render: _.debounce(function() {
            this.graphs.each(function(graph) {
                // Big giant switch to select/create $container!?

                graph.render().$el.appendTo($container);
            });
            $(window).on('resize', this.draw);

            return this;
        }, 100)
    });
});
share|improve this question

closed as off-topic by Jamal Feb 21 at 17:43

If this question can be reworded to fit the rules in the help center, please edit the question.

    
That's a beautiful example of some libraries that I have never used before. I am curious though, what do the double braces do in var properties = {{ properties | json_encode | raw }}? –  Dan Ross Jun 13 '13 at 2:16
    
Oh, I should have included the <?php tag in that snippet. The {{ var }} is how Twig denotes variables to be interpolated in a PHP template. twig.sensiolabs.org. I'm bootstrapping my data so you don't have to wait for an AJAX request after pageload. –  John Syrinek Jun 13 '13 at 14:27
    
This question appears to be off-topic because it is seeking a design review as opposed to a code review. –  Jamal Feb 21 at 17:43

1 Answer 1

up vote 0 down vote accepted

I ended up using the decorator pattern. The report view and each graph object now implement a decorate() method. When the report view's decorate() is called, it iterates over each graph, calling the graph's decorate(). The latter method receives the report view as a parameter, and this provides access to convenience methods for creating/retrieving DOM elements for the graph to attach itself to.

New report view:

define([
    '/script/graphs/common.js',
    '/script/graphs/facebook.js',
    '/script/graphs/twitter.js',
    '/script/graphs/youtube.js',
    '/script/graphs/analytics.js'
], function(common, facebook, twitter, youtube, analytics) {
    'use strict';

    return Backbone.View.extend({
        initialize: function() {
            this.graphs = this.options.graphs;
        },

        defaults: {
            edit_mode: false,
            graphs: new Backbone.ChildViewContainer()
        },

        decorate: function() {
            this.graphs.each(_.bind(function(graph) {
                graph.decorate(this);
            }
        }, this),

        render: _.debounce(function() {
            this.graphs.each(function(graph) {
                graph.render();
            });    
            return this;
        }, 100),

        getPage: function(name) {
            var $page = this.$el.find('.page.' + name);
            if (!$page.length) {
                $page = $('<div/>').addClass('page ' + name).appendTo(this.$el);
            }
            return $page;
        }
    });
});

Example graph object:

define([], function() {
    'use strict';

    return Backbone.View.extend({
        decorate: function(report_view) {
            $page = report_view.getPage('overview');
            this.$el.appendTo($page);
        },

        render: function() {
            // Render graph to this.$el
        }
    });
});
share|improve this answer

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