HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Xerox Parc MVC implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mvcimplementationparcxerox

Problem

I've been trying to understand the original MVC implementation (the Xerox Parc's one). I'm sure it has flaws, but it's simple code to practice/learn the original MVC.

Working example

View (index.html):


    
        
        Person
    
    
        
        Birth Date: 
        Age: 
        
        
        
        
    


Model (model-person.js):

var ModelPerson = (function () {
    'use strict';

    var ModelPerson = function (name, birthDate) {
        this.name = name;
        this.birthDate = birthDate;
    };

    ModelPerson.prototype.getName = function () {
        return this.name;
    };

    ModelPerson.prototype.setName = function (name) {
        this.name = name;
    };

    ModelPerson.prototype.getBirthDate = function () {
        return this.birthDate;
    };

    ModelPerson.prototype.setBirthDate = function (birthDate) {
        this.birthDate = birthDate;
    };

    ModelPerson.prototype.getAge = function () {
        var dateDifference = new Date(Date.now() - this.birthDate.getTime());
        return Math.abs(dateDifference.getFullYear() - 1970);
    };

    return ModelPerson;
}());


Controller (controller-person.js):

```
var ControllerPerson = (function () {
'use strict';
var controller;

var ControllerPerson = function (modelPerson) {
controller = this;
controller.modelPerson = modelPerson;
};

ControllerPerson.prototype.initialize = function () {
document.getElementById('person-name').addEventListener('blur', function () {
controller.modelPerson.setName(this.textContent);
controller.updateView();
});

document.getElementById('person-birth-date').addEventListener('blur', function () {
controller.modelPerson.setBirthDate(new Date(this.textContent));
controller.updateView();
});

controller.updateView();
};

ControllerPerson.prototype.updateView = function () {
document.getElement

Solution

I am assuming you are used to getters/setters because you are experienced in another language, please don't do this. Performance matters, there is no good reason to download all those functions for no good reason, model-person.js should be

var ModelPerson = (function () {
    'use strict';

    var ModelPerson = function (name, birthDate) {
        this.name = name;
        this.birthDate = birthDate;
    };

    ModelPerson.prototype.getAge = function () {
        var dateDifference = new Date(Date.now() - this.birthDate.getTime());
        return Math.abs(dateDifference.getFullYear() - 1970);
    };

    return ModelPerson;
}());


Also, updateView does not belong in the controller, it should be part of your view class which should contain all view related logic.

Furthermore, I think in the original pattern, the model actually updates the view, meaning that the controller should not be done the one doing controller.updateView();, that should be handled implicitly by controller.modelPerson.setName(this.textContent);

Personally, I like your approach better, I think the controller should decide when the UI gets updated.

Finally, there is the matter of the element id's like person-name, you use them more than once so you should use a constant. Now the question is, should this constant be defined in Model, View or Controller ? I personally define these id's in the View class and then the controller asks the View what the element is, so that it can attach the listener. I dont think the MVC pattern goes into this detail, so it's up to you.

Other than that, your code is very readable and easy to follow.

Code Snippets

var ModelPerson = (function () {
    'use strict';

    var ModelPerson = function (name, birthDate) {
        this.name = name;
        this.birthDate = birthDate;
    };

    ModelPerson.prototype.getAge = function () {
        var dateDifference = new Date(Date.now() - this.birthDate.getTime());
        return Math.abs(dateDifference.getFullYear() - 1970);
    };

    return ModelPerson;
}());

Context

StackExchange Code Review Q#43265, answer score: 2

Revisions (0)

No revisions yet.