6
\$\begingroup\$

I would like advice if this is a good implementation of an application for maintaining information about people (first name, last name, and age).

The index.html contains an empty div and 2 templates (1 for listing people and the other for entering/editing a user):

<div id="app">
</div>

<script type="text/template" id="listTemplate">
<!-- Iterates over people and makes a table. -->
</script>

<script type="text/template" id="editTemplate">
<!-- Displays a form which if for existing user will fill the values. -->
</script>

In my starting point JS file I put into a namespace 'people.global' a bunch of things that I use throughout the application:

$(function() {

    people.global = {
        people: new people.collections.PeopleCollection()
    };

    people.global = $.extend(people.global, {
        listView: new people.views.ListView(),
        editView: new people.views.EditView(),
        router: new people.Router()
    });

    people.global.people.fetch();

    Backbone.history.start();

});

The router demonstrates the basic idea which is depending on the screen that should be shown it takes the View's el and draws it in the #app element:

var people = people || {};

people.Router = Backbone.Router.extend({

    $app: $('#app'),

    routes: {
        '': 'list',
        'edit': 'edit',
        'edit/:id': 'edit',
        'delete/:id': 'delete'
    },

    list: function() {
        this.$app.html(people.global.listView.render().el);
    },

    edit: function(id) {
        people.global.editView.setId(id);
        this.$app.html(people.global.editView.render().el);
    },

    delete: function(id) {
        var person = people.global.people.get(id);
        person.destroy();
        this.navigate('', {trigger: true, replace: true});
    }

});

Below is the view for editing/adding a person:

var people = people || {};
people.views = people.views || {};

people.views.EditView = Backbone.View.extend({

    template: _.template($('#editTemplate').html()),

    setId: function(id) {
        this.id = id;
    },

    edit: function() {
        var person;

        if (this.$("#id").val() !== '') {
            person = people.global.people.get(this.id);
            person.set({
                'first': this.$("#first").val(),
                'last': this.$("#last").val(),
                'age': this.$("#age").val()
            });

            person.save();
        } else {
            people.global.people.create({
                first: this.$('#first').val(),
                last: this.$('#last').val(),
                age: this.$('#age').val()
            });         
        }

                people.global.router.navigate('', {trigger: true, replace: true});
    },

    render: function() {
        var person,
            that = this;

        if (this.id) {
            person = people.global.people.get(this.id); 
            this.$el.html(this.template(person.toJSON()));
        } else {
            this.$el.html(this.template({}));
        }

        this.$('button').click(function() {
            that.edit();
        });

        return this;
    }

});

Is this a good design choice?

Is there something that can be done better?

\$\endgroup\$
1
\$\begingroup\$

All in all, this looks okay to me, proper naming, nothing bizarre, ( no commenting ).

The only thing that bothers me is that you call .setId(id); on the editView.

editView should have a data model with 1 person, and the .setId() should be called on that data model ( perhaps your function setId() does exactly that ? )

(Deleted race conditions, it not relevant)

\$\endgroup\$
2
  • \$\begingroup\$ I don't know if this is good. As you can see in EditView edit() method checks for this.id and takes appropriate action. \$\endgroup\$ – SBel Mar 28 '14 at 18:26
  • \$\begingroup\$ I removed my race condition remark, I still feel it's odd that the view get's the data itself instead of getting the data from the controller, or keeping a reference to this.person which could be set in setId \$\endgroup\$ – konijn Mar 28 '14 at 19:09

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.