CODEWITHSUNDEEP
javascriptjquerybackbone.js
Ivan
Ivan

Reputation: 157

Destroy() doesn't work properly

I'm writing basic to-do list using Backbone.js. Every input adding as a model to collection. Listening for 'add' on collection and rendering newly added model (appending li with 'task' to ul). Then by double-clicking on item I'm retrieving html() of it and in a loop comparing it to corresponding attribute in a model. When it catch the right model - destroying the model (should be deleted from a collection accordingly). But some issue occuring in console, it says

Uncaught TypeError: Cannot read property 'toJSON' of undefined

and adding some buggy effect (not everytime can delete item by the first dblckick). If anyone can point the problem out it would be greatly appreciated! Here's code 

    var Model = Backbone.Model.extend({
    default: {
        task: '',
        completed: false
    }
});

var Collection = Backbone.Collection.extend({
    model: Model
});

var ItemView = Backbone.View.extend({
    tagName: 'li',
    render: function () {
        this.$el.html(this.model.toJSON().task);
        return this;
    }
});

var TodoView = Backbone.View.extend({
    el: '#todo',

    initialize: function () {
        this.collection = new Collection();
        this.collection.on('add', this.render, this);
    },

    events: {
        'click .add': 'add',
        'dblclick li': 'destroy',
        'keydown': 'keyEvent'
    },

    add: function () {
        this.collection.add(new Model({ //adding input as an model to collection
            task: this.$el.find('#todo').val(),
            completed: false
        }));

        this.$el.find('#todo').val(''); //clearing input field
        this.$el.find('#todo').focus(); //focusing input after adding task
    },

    keyEvent: function (e) {
        if (e.which === 13) {
            this.add();
        }
    },

    destroy: function (e) {
        // console.log(this.collection.toJSON());
        this.collection.each(function (model) {
            if ($(e.target).html() === model.toJSON().task) {
                model.destroy();
            }
        });
        e.target.remove();
        // console.log(this.collection.toJSON());
    },

    render: function (newModel) {
        var self = this,
            todoView;

            todoView = new ItemView({
                model: newModel
            });

            self.$el.find('.list').append(todoView.render().el); 

        return this;
    }

});

var trigger = new TodoView();

And here's http://jsbin.com/ciwunizuyi/edit?html,js,output

Upvotes: 2

Views: 257

Answers (1)

Andrew Burgess
Andrew Burgess

Reputation: 1797

The problem is that in your destroy method, you find the model to destroy by comparing the task property of the models. If you have multiple models with the same task property, you'll get the error. The actual error occurs because you're removing items from the collection while iterating over it.

Instead of comparing the task property, you could use the cid (client id) property that Backbone gives all models. One way to do this would be this:

  • When rendering an ItemView, use jQuery's data method to store the cid with the view element (alternatively, use a custom data attribute)

    this.$el.data('cid', this.model.cid);

  • In the destroy function, get the cid property from the view element, and use it to find the right model in the collection (you can use the collection's get method here):

    destroy: function (e) {
    var id = $(e.target).data('cid');
    var model = this.collection.get(id);
    model.destroy();
    e.target.remove();
},

Adding a unique attribute to the DOM element is only one way to solve this problem. One, much better, alternative would be to listen for the double-click event from the ItemView class itself. That way, you would always have a reference to this.model.

EDIT: This shows the code above in action: http://jsbin.com/nijikidewe/edit?js,output

Upvotes: 1

Related Questions