3
\$\begingroup\$

I would really appreciate a review of my interpretation of MVC here.

It's as bare bones and clear as possible. I've based it on some in-depth reading but I don't know if I have understood everything correctly!

  1. For example, how have I wired the model onChange to the controller?
  2. Where/when should the view be initialised, and does it follow to bind DOM events in that init function?
  3. Am I right to pass a path into the controller, thus binding view to one node in the model?

MVC Objects

(function(window, document, undefined){

    //Add namespace to window
    window.mvc = {
        Model: Model,
        View: View,
        Controller: Controller
    }

    /*----------
       Objects
      ---------*/
    function Model(tree){
    //Constructor
        var model = this;
        model.tree = tree || {};
        model.cbFns = []; //onChange callbacks
    }

    Model.prototype = {

        set: function(props){
        //Set a node value
            var model = this,
                cbFns = model.cbFns;
            mixin(model.tree, props);

            //Run any registered callbacks
            var i = cbFns.length;
            while(i--)cbFns[i].call();
        },
        get: function(p){
        //Get a node value
            var model = this;
            return getNode(model.tree, p);
        },
        toJSON: function(){
        //Render tree as JSON
            var model = this;
            return JSON.stringify(model.tree);
        },
        onChange: function(cbFn){
        //Register a change callback
            var model = this;
            model.cbFns.push(cbFn);
        }
    }

    function View(props){ //props must contain 'render' method
    //Constructor
        var view = this;
        mixin(view, props);
    }

    function Controller(props){ //props must contain 'model', 'view' objects and 'path' string
    //Constructor
        var controller = this;
        mixin(controller, props);
        controller.view.controller = controller; //Ref to controller on view

        //Add callback to model
        controller.model.onChange(function(){
            controller.get();
        });
    }

    Controller.prototype = {

        set: function(val){
        //Write to model
            var controller = this,
                setArgs = {};
            setArgs[controller.path] = val;
            controller.model.set(setArgs); //Write to model
        },
        get: function(){
        //Update view from model
            var controller = this;
            controller.view.render(controller.model.get(controller.path));
        }
    }

    /*--------
       Utils
      -------*/
    var pathSeparator = ".";

    function setNode(tree, pathStr, value){
    //Set node at path string
        var pathArr = pathStr.split(pathSeparator),
            prop = pathArr.pop(),
            parentPathStr = pathArr.join(pathSeparator),
            currNode = parentPathStr && getNode(tree, parentPathStr) || tree;
        currNode[prop] = value;
    }

    function getNode(tree, pathStr){
    //Get node from path string
        var pathArr = pathStr.split(pathSeparator),
            currNode = tree,
            i = pathArr.reverse().length;
        while(i--)currNode = currNode[pathArr[i]];
        return currNode;
    }

    function mixin(ob1, ob2){
    //Add/overwrite all properties right to left
        for(var p in ob2){
            if(ob2.hasOwnProperty(p)){
                setNode(ob1, p, ob2[p]);
            }
        }
    }

})(window, document);

Application

(function(window, document, undefined){

    var mvc = window.mvc,
        form = document.getElementById("userForm");

    //Create user model
    var userModel = new mvc.Model({
            profile: {
                name: "Prof Farnsworth",
                age: "160",
                region: "Space"
            }
        });

    function createInputField(model, path){
    //Creates MVC input fields

        //View is the same for all input fields
        var inputView = new mvc.View({
            init: function(){
                var view = this,
                    controller = view.controller;

                view.domEl = createEl("input", form);
                view.domEl.onblur = function(e){
                    controller.set(this.value);
                }
                controller.get();
            },
            render: function(data){
                var view = this;
                view.domEl.value = data;    
            }
        });

        //Create controller
        new mvc.Controller({
            view: inputView,
            model: model,
            path: path
        });

        //Initialise view
        inputView.init();
    }

    //Fields
    createInputField(userModel, "profile.name"),
    createInputField(userModel, "profile.age");
    createInputField(userModel, "profile.region");

    //Button to update model
    var testBtn = createEl("button", document.body, "Update model");
    testBtn.onclick = function(e){
        userModel.set({"profile.name":"Zoidberg"});
    }

    //Button to log model
    var logBtn = createEl("button", document.body, "Log model to console");
    logBtn.onclick = function(e){
        console.log( userModel.toJSON() ); 
    }


    /*--------
       Utils
      -------*/
    function createEl(type, parent, innerHTML){
        var el = document.createElement(type);
        if(parent)parent.appendChild(el);
        if(innerHTML)el.innerHTML = innerHTML;
        return el;
    }

})(window, document);
\$\endgroup\$
0
1
\$\begingroup\$

* model.cbFns = []; //onChange callbacks

I would rename this to callBacks, cbFns just feels wrong

    set: function(props){
    //Set a node value
        var model = this,
            cbFns = model.cbFns;
        mixin(model.tree, props);

        //Run any registered callbacks
        var i = cbFns.length;
        while(i--)cbFns[i].call();
    },

calling the callbacks seems non-intuitive, how about

    set: function(props){
        var model = this;
        mixin(model.tree, props);
        model.cbFns.forEach( function(f){ f(); } )  
    },

These 2 lines:

var model = this; 
var view   = this; 

Not sure this is useful, especially in 2 liner functions

From a design perspective, it seems as if you create a View per input field. Usually there is 1 view with each field being a child of it? Also the logic within createInputField() really should not be part of Application?

A view should be able to generate the entire model, so it should know how to do all the work.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for feedback I will try your suggested changes! The logic in createInputField()... I figured that different views will need different render() and init() functions. So say, input fields need an onblur event on the DOM element but other views might have onclick. So I'm supplying them when creating the view, to keep the MVC logic generic. Is there a better way of initialising the view? \$\endgroup\$ – user27768 Jul 30 '13 at 11:32

Your Answer

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