Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm working on my larger project to date, and on top of that I'm using underscore for the first time. All is working perfectly but I'd like some criticism on the use of underscore templating and underscore after function.

The goal of the app is to generate on the dom a messaging interface starting from an empty div. It gets initialized by passing the jQuery selector of that empty div.

I took out most of the code and left what I'd like to be reviewed the most, hope the flow of the app is quite clear anyway.

(function($) {
$(document).ready(function() {

var mssg_tmpl = {
        msg_container : ' \
    <div class="row-fluid msg-container well" id="msg-<% id %>"> \
        some html \
    </div>',

        msg_reply : ' \
    <div class="reply well message"> \
        some html \
    </div>'

};

mssg = {
    init : function(selector) {
        this.$container = $(selector);
        this.tmpl = {};         // List of compiled templates functions
        this.compileTemplates(mssg_tmpl);
        this.messages = [{      // THIS IS HARDCODE TO ILLUSTRATE BASIC STRUCTURE
            id: 1,              // Thread opener
            children: [2,3,4]   // Replies
        }, {
            id: 5, 
            children: [6,7,8]
        }];

        for ( var i in this.messages) {
            this.$container.append(this.tmpl.msg_container(this.messages[i])); // fn msg_container is a compiled template
            this.messages[i].$container = $('#msg-' + this.messages[i].id);
            this.fetchChildren(this.messages[i]);
        }

        this.bindEvents();
        this.collapseMessages();
        return this;
    },

    compileTemplates : function(templates) {
      for ( var k in templates) {
        this.tmpl[k] = _.template(templates[k]);
      }
    },

    fetchChildren : function(message) {

        var renderChildren = _.after(message.children.length, function() {
            children = _.sortBy(children, 'id'); // Sort replies (oldest first)
            for ( var i in children) {
                message.$container.find('div.msg-reply-container').before(mssg.tmpl.msg_reply(children[i])); // fn msg_reply is a compiled template
            }
        });

        var children = [];

        for ( var i in message.children) {
            $.ajax({
                url : 'getmessage.htm?messageId=' + message.children[i]
            }).done(function(response) {
                children.push(response);
                renderChildren();
            });
        }

    },
};

mssg = mssg.init('#messaging');

});

})(jQuery);
share|improve this question
add comment

1 Answer

up vote 2 down vote accepted

Since you asked to review only the underscore part,

Just had one remark that I would suggest to move the templates to a script tag "text/template" rather than defining them as a variable.

eg:

<script type="text/template" id="msg_container">
    <div class="row-fluid msg-container well" id="msg-<% id %>">
        some html
    </div>
</script>

<script type="text/template" id="msg_reply">
    <div class="reply well message">
        some html
    </div>
</script>

Compile and use the same like below,

var msg_container = _.template($('#msg_container').html());
var msg_reply = _.template($('#msg_reply').html());

Though you asked to review only the '_' part but couldn't resist asking this question, From your example snippet I see 6 potential AJAX calls based on the loops. Wouldn't there be a performance problem since it completely relies on the number of messages and replies and what if the messages and replies are more or I could be missing the overall picture :)

share|improve this answer
1  
Hi thanks for taking the time to read and comment! I know the best practice would be to move templates to a script tag but the goal is to make this a library than can be imported without much hassle into all the apps my company works on, so it's essential that who implements it in their code only has to include a js file, a css file, and an empty div where the app will be initialized. I am considering moving templates to a separate js file which could be a little better from the present solution. –  David Fregoli Feb 28 '13 at 15:27
    
@DavidFregoli I would keep templates as much as possible out of the library. Separation of concerns. –  Dmitri Zaitsev Dec 10 '13 at 14:24
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.