Order a delicious pie here

Problem

For a quick summary: I've created this internal web application, and I've hit a point where I can really see the mess I've made. I need some help separating the logic, the view, and the data.

More detail: Over the past few months, I've been doing all I can to learn more about JavaScript built web sites/applications. I've created the below code, and it seems as though any additions are just ruining the entire thing. This is the fourth time I've started from scratch on this, and I can't get a product I really like.

Now, it works as it should, I just don't like the way it's built. I tried using Angular.js, but that was over-kill for this single page app (plus working with the routing was nightmarish). Now I've just created this mess of a main.js file, and it needs refactoring.

It'd be great if we could avoid suggesting tools requiring node.js.

index.html

```

Pies

Pies

New Order?
Successfully created!
Something went wrong, try again later.

Customer name

Due date (optional)

Payments

Orders
Successfully paid!
Something went wrong, try again later.

Name
Priority
Flavor

Solution

First of all I'd like to congratulate you on the HTML part, that one looks clean and takes almost all best practices into consideration. I say 'almost' since ... yeah well ... these days people argue you should put script tags before the `

 tag. This to avoid http stalling the reflow of the browser. Oh well, for simple applications leave it like that. If you want to scale up and add more libraries in the future, you might consider moving the scripts to the bottom.

Next one then, the JavaScript part. If you say it works, well done!
You say it looks ugly ... ? Do you also know why? Let me sum that up for you just to make sure we're on the same page:

logic mixed with strings is a "nono" if you want to write beautiful code => configurable strings



the templating is kinda hard-coded into the logic => templating system



the use of globally defined functions => closure



the amount of iterations (not sure if I can reduce them, we'll see along the way) => best practices in DOM appending



no function describes what the "main" part is (this becomes essential once you scale up) => modular approach



You don't want to use some additional libraries for this piece of code? I totally agree! You sound like a good decision maker, now you need a little push in the right direction.

So ... I've been spreading my logic here and there over stackoverflow/codereview and I believe it will help you too. Please read them as I'm not going to copy/paste the whole idea again. I will provide refactored code and the extra information I'm sure you can take that from some of those answers I've linked.

I use Re-Sharper for JavaScript and I like it "green" (read: jshint valid) so let me tell you what goes wrong even though your code "works":

ln 24 & 45: Declaration hides parameter data from outer scope



ln 32: Use of an implicitly declared global variable 'fetchOrders' (assuming this is a  false positive)



ln 95 102 104: Duplicate declaration



ln 102: Value assigned is not used in any execution path



ln 110: Not all code paths return a value



So this is how I do it. Take the time to compare the approach below with my previously posted answers. It's actually the same stuff over and over again. Once you get the hang of it, you'll notice the benefit of object literals and how to extend/configure YOUR OWN library.

``
window.DeliciousPie = (function ($, project) {

// 1. CONFIGURATION
var cfg = {
cache: {
container: '[data-component="orderpie"]',
flavors: '.flavorSelector',
flavorsTable: '#flavorsTableContainer tbody',
flavorForm: '#newFlavorForm',
flavorFormInputs: 'input[type=text]',
flavorSuccess: '#newFlavorContainer > span.success',
ordersTable: '#ordersTableContainer tbody',
orderForm: '#newOrderForm',
orderSuccess: '#newOrderContainer > span.success',
orderFail: '#newOrderContainer > span.fail',
dueDate: '#dueDate',
customerName: '#customerName',
paymentForm: '.payment-form',
paymentSuccess: '#ordersContainer > span.success',
paymentFail: '#ordersContainer > span.fail',
formTarget: 'div.button'
},
data: {
hash: 'hash'
},
events: {
click: 'click'
},
tpl: {
flavor: '{{flavor}}',
paymentForm: 'Paid',
priority: '',
orderRow: '{{name}}{{priority}}{{flavor}}{{paymentForm}}',
flavorRow: 'Delete'
},
ajaxOptions: {
get: {
url: 'scripts/appdata.json',
dataType: 'json'
},
post: {
flavor: {
url: 'scripts/server.php',
data: {
action: 'newFlavor'
}
},
order: {
url: 'scripts/server.php',
data: {
action: 'newOrder'
}
},
pay: {
url: 'scripts/server.php',
data: {
action: 'payOrder'
}
}
}
},
priorityOptions: {
colors: ['#A30E0E', '#FF9401', '#6FBF0D'],
marks: [172800, 64800, 0]
}
};

// 2. ADDITIONAL FUNCTIONS
/**
* @description Render html template with json data
* @see handlebars or mustache if you need more advanced functionality
* @param {Object} obj
* @param {String} template : html template with {{keys}} matching the object
* @return {String} template : the template string replaced by key:value pairs from the object
*/
function renderTemplate(obj, template) {
var tempKey, reg, key;

for (key in obj) {
if (obj.hasOwnProperty(key)) {

Code Snippets

window.DeliciousPie = (function ($, project) {

    // 1. CONFIGURATION
    var cfg = {
        cache: {
            container: '[data-component="orderpie"]',
            flavors: '.flavorSelector',
            flavorsTable: '#flavorsTableContainer tbody',
            flavorForm: '#newFlavorForm',
            flavorFormInputs: 'input[type=text]',
            flavorSuccess: '#newFlavorContainer > span.success',
            ordersTable: '#ordersTableContainer tbody',
            orderForm: '#newOrderForm',
            orderSuccess: '#newOrderContainer > span.success',
            orderFail: '#newOrderContainer > span.fail',
            dueDate: '#dueDate',
            customerName: '#customerName',
            paymentForm: '.payment-form',
            paymentSuccess: '#ordersContainer > span.success',
            paymentFail: '#ordersContainer > span.fail',
            formTarget: 'div.button'
        },
        data: {
            hash: 'hash'
        },
        events: {
            click: 'click'
        },
        tpl: {
            flavor: '<div class="button" style="width: 60%">{{flavor}}</div>',
            paymentForm: '<form class="payment-form"><input type="text"/><div class="button paid">Paid</div></form>',
            priority: '<div class="priority" style="background: "{{priority}}"></div>',
            orderRow: '<tr data-hash="{{hash}}"><td>{{name}}</td><td>{{priority}}</td><td>{{flavor}}</td><td>{{paymentForm}}</td></tr>',
            flavorRow: '<tr><td><input type="text" value="{{flavor}}"/></td><td><span>Delete</span></td></tr>'
        },
        ajaxOptions: {
            get: {
                url: 'scripts/appdata.json',
                dataType: 'json'
            },
            post: {
                flavor: {
                    url: 'scripts/server.php',
                    data: {
                        action: 'newFlavor'
                    }
                },
                order: {
                    url: 'scripts/server.php',
                    data: {
                        action: 'newOrder'
                    }
                },
                pay: {
                    url: 'scripts/server.php',
                    data: {
                        action: 'payOrder'
                    }
                }
            }
        },
        priorityOptions: {
            colors: ['#A30E0E', '#FF9401', '#6FBF0D'],
            marks: [172800, 64800, 0]
        }
    };

    // 2. ADDITIONAL FUNCTIONS
    /**
     * @description Render html template with json data
     * @see handlebars or mustache if you need more advanced functionality
     * @param {Object} obj
     * @param {String} template : html template with {{keys}} matching the object
     * @return {String} template : the template string replaced by key:value pairs from the object
     */
    function renderTemplate(obj, template) {
        var tempKey, reg, key;

        for (key in obj) {
            if (obj.hasOwnProperty(key)) {

Context

StackExchange Code Review Q#77843, answer score: 13

Revisions (0)

No revisions yet.