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

Basic singlepage blog application

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

Problem

I've started learning Mithril framework, which looks pretty awesome. I've created a basic blog app and have a few questions about code style and structure.

```
var app = {};

app.PostList = function(list) {
return m.prop(list || []);
};

app.vm = {};

app.Post = function(data) {
var data = data || {};
this.id = m.prop(data.id);
this.title = m.prop(data.title);
this.content = m.prop(data.content);
};

app.vm.init = function() {
var self = this;
self.posts = new app.PostList();
m.request({method: 'GET', url: '/posts', background: true, type: app.Post})
.then(self.posts)
.then(m.redraw);
};

app.controller = function() {
app.vm.init();
};

app.view = function(ctrl) {
return m('body', [
m('h1', 'My blog'),
m('ul',
app.vm.posts().map(function(el){
return m('li', [
m('a[href="/posts/'+el.id()+'"]', {config: m.route}, JSON.stringify(el))
]);
})
),
m('a[href="/posts/add/"]', {config: m.route}, 'add new post')
]);
};

var add = {};

add.newPost = function(data) {
data = data || {};
this.title = data.title || '';
this.content = data.content || '';
};

add.controller = function() {
var post = new add.newPost();
this.post = post;
this.create = function(e) {
e.preventDefault();
m.request({
method: 'POST',
url: '/posts',
data: post
}).then(function() {m.route('/');});
};
};

// data binding helper function
// http://lhorie.github.io/mithril-blog/asymmetrical-data-bindings.html
function formBinds(data) {
return function(e) {data[e.target.name] = e.target.value;};
}

add.view = function(ctrl) {
return m('body', [
m('form', {
onsubmit: ctrl.create,
onchange: formBinds(ctrl.post)
}, [
m('a[href="/"]', {config: m.route}, 'All Posts'),
m('br'),
m('input[type=text][name=title][placeholder="post title"][required]'),
m('br'),
m('textarea[name=content][placeholder="post content"][required]'),

Solution

I noticed you're using app as a module name (which is understandable, since it's what the tutorial in the Mithril site does now). For real apps, you should try to name your modules more descriptively. For example, if the module displays a list of Posts, you might want to call it posts or postList.

Instead of calling m.request from vm, I normally pull the request out into a method on the model class, for example:

var Post = function(data) {
  var data = data || {};
  this.id = m.prop(data.id);
  this.title = m.prop(data.title);
  this.content = m.prop(data.content);
}
Post.list = function() {
  return m.request({method: 'GET', url: '/posts', type: app.Post})
}


You might optionally want to wrap m.request itself in another method, e.g.

var req = function(args) {
  return m.request(args)
}
/*...*/
Post.list = function() {
  return req({method: 'GET', url: '/posts', type: app.Post})
}


This lets you easily configure requests globally (e.g. if you need to set HTTP headers on all requests)

On a side note, I generally don't recommend using the background:true + m.redraw combo (especially on first render), unless you have a really good reason. When you do that, you're allowing your view to run early, which means all of your asynchronous data needs to be considered nullable, which in turn means you need more defensive code in the view. It's also a band-aid solution if the root of the problem is a slow web service.

Re: this comment: "TODO: add request error handler (how to?)"

Assuming you created a model method for posting as well:

Post.save = function(args) {
  return m.request({method: 'POST', url: '/posts', data: args, unwrapError: unwrap})
}


Then you can use the rejection handler of the promise to bind errors to a getter-setter

self.error = m.prop("")
self.add = function() {
  /*...*/
  Post.save()
    .then(null, self.error)
}


You can define the unwrapError function to drill down into a response to get the desired error message string out of the JSON structure. (There's also the extract option which lets you handle the response at the XHR level)

So, if your response looks like {msg: "something died"}, then you could write unwrapError: function(e) {return e.msg}, and then to display the error, you would simply call vm.error() in the view, which would return the "something died" string.

I also normally use a layout wrapping module, which looks something like this:

var layout = {}

layout.controller = function(module) {
  this.controller = new module.controller
  this.view = module.view
}

layout.view = function(ctrl) {
  return m(".layout", [
    ctrl.view(ctrl.controller)
  ])
}

layout.wrap = function(routes) {
  var map = {}
  Object.keys(routes).map(function(r) {
    map[r] = {
      controller: function() {
        return new layout.controller(routes[r])
      },
      view: layout.view
    }
  })
  return map
}


Then I can wrap modules like this:

m.route(document, "/", layout.wrap({
  "/": posts
}))


With this, the inner modules don't need to redefine the layout stuff all the time.

Regarding usage of vm versus using the controller, you only really need view-models if you need your modules to be globally accessible by other modules (this usually only happens when you have a deep hierarchy of sub-templates).

Most of the time, using fat controllers yield shorter, clearer code than using view-models (see app.vm.init + app.controller vs add.controller).

Still on the topic of controllers, one technique that I've seen in angular styleguides is to code controllers like this:

foo.controller = function() {
  var foo = {bar: 1}
  return {
    foo: foo
    doStuff: doStuff
  }

  function doStuff() {
    alert(foo.bar)
  }
}


This style avoids issues with this scoping and makes it easier to scan the public signature of the controller.

When using m.route, I recommend that you bind to document.body instead of document, so that you can put stylesheets in `` and take advantage of the browser downloading/rendering optimizations.

Other than those, the code looks fairly well organized. The views are well written, and the code is easy to read, overall.

Code Snippets

var Post = function(data) {
  var data = data || {};
  this.id = m.prop(data.id);
  this.title = m.prop(data.title);
  this.content = m.prop(data.content);
}
Post.list = function() {
  return m.request({method: 'GET', url: '/posts', type: app.Post})
}
var req = function(args) {
  return m.request(args)
}
/*...*/
Post.list = function() {
  return req({method: 'GET', url: '/posts', type: app.Post})
}
Post.save = function(args) {
  return m.request({method: 'POST', url: '/posts', data: args, unwrapError: unwrap})
}
self.error = m.prop("")
self.add = function() {
  /*...*/
  Post.save()
    .then(null, self.error)
}
var layout = {}

layout.controller = function(module) {
  this.controller = new module.controller
  this.view = module.view
}

layout.view = function(ctrl) {
  return m(".layout", [
    ctrl.view(ctrl.controller)
  ])
}

layout.wrap = function(routes) {
  var map = {}
  Object.keys(routes).map(function(r) {
    map[r] = {
      controller: function() {
        return new layout.controller(routes[r])
      },
      view: layout.view
    }
  })
  return map
}

Context

StackExchange Code Review Q#68447, answer score: 5

Revisions (0)

No revisions yet.