Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Technical review from Jeremy Ashkenas #342

Closed
addyosmani opened this Issue Feb 22, 2013 · 10 comments

Comments

Projects
None yet
3 participants
Owner

addyosmani commented Feb 22, 2013

(minus some general book/market comments)

Chapter-by-chapter Review

Prelude

The first intro sentence: "How do you write an organized, maintainable
application with JavaScript?" is a bit weak. Pick something a little stronger,
and slam the point home.

"work your way through the internals, practicals" ... it looks like there's
going to be a large number of typos and grammaros in here, so I'll ignore them
going forward, and assume that O'Reilly copyeditors will catch them in due time.

"The challenge with this approach is that it doesn’t take long to get lost in a
nested pile of callbacks and DOM elements without any real structure in place."
A nice start, but an explanation of the problem could use a bit more exposition
here. I bet a brief desription of the kind of mess you can find yourself in would
really resonate with a lot of the reading audience for this book. -- Especially
if you mention how easy it becomes to tangle up your business logic with the
specifics of how your interface works.

What is MVC?

Nice explanation, but it would be worth hitting home that the entire central
point of MVC is separating your business logic from your user interface. Hence,
MV* makes sense -- it's all about establishing solid and mutually independent
foundations for your logical layer, and your interface layer.

When Do I Need A JS MVC Framework?

"GMail and Google Docs" ... non-Google examples would be welcome in this
sentence too.

Why Consider Backbone.js

This series of sections is getting a little bit sales pitch-y. I'd try to find
a way to make it less used car salesman, and more an explanation of how
Backbone's pieces already mirror the functional pieces that currently make up
your ad-hoc web application.

The "Does the following describe you?" graf is very jargon-heavy -- which would
be nice to be reduced particularly because some of the jargon is incorrect:

"It should be imperative, allowing Views to update themselves when Models
change." ... doesn't make any sense.

Smalltalk-80 MVC

Nice explanation.

MVC Applied to the Web

This section could use a bit of love. At the moment, it probably confuses the
reader more than it enlightens.

That said, an explanation of how traditional (read, Rails) server-side MVC
differs architecturally from client-side MVC would be welcome, and would fit
nicely in here. It can be simple. Just emphasise how even for the server-side
workflow -- receiving a request for a URL, and baking out an HTML page as a
response -- separating your business logic from your interface has many benefits.
And in the same way that keeping your UI cleanly apart from your data models
is useful in JavaScript, keeping your UI cleanly apart from your database records
is useful in server-side frameworks...

MVC In the Browser

This is a strange section that doesn't really explain what it's trying to say,
at the moment. I'd take another pass at it -- again, trying to draw parallels
with the previous section.

Client-Side MVC - Backbone Style

I'd avoid using the -min.js versions of files for all examples. It'll just
make for headaches trying to learn and debug for beginners.

"Instead, the Controller responsibility is addressed within the View." If you're
going to make this point, which is a fine one, it's also worth mentioning that
the traditional controller role (Smalltalk-80 style) is performed by the template,
not by the Backbone.View.

Implementation Specifics

"A Model may also have single or multiple Views observing it." It's not explained
here what this means, and I think you need to explain "observe" if you're going
to use it this early.

For Collections, it's also worth mentioning that they're useful for performing
any aggregate computations across more than one model.

"In JavaScript applications state has a specific meaning, typically referring to
the current state of a view or sub-view on a user’s screen at a fixed time."
Not true at all. "State" means the same thing in older texts as it does now in
common parlance. I'd kill this graf.

You mention Handlebars and Mustache, then immediately say <%= for interpolation,
which will be confusing. Change one or the other.

Re: the "Controllers" mini-section ... I think you've already covered this up
above, and it doesn't need repeating here.

What does MVC give us?

"To summarize, the separation of concerns in MVC facilitates modularization
of an application’s functionality and enables:" "facilitates modularization"?
Way too jargony. Emphasize that it helps you keep your logic separate from your
user interface, making it easier to change and maintain both.

"Doesn’t support deeply nested Models, though there are Backbone plugins
such as Backbone-relational which can help" I'd be careful here, as in my
opinion, the usual approach to nested
models: http://backbonejs.org/#FAQ-nested, is way less error-prone than that
particular plugin.

Backbone Basics / Models

"Model.set() allows us to pass attributes into an instance of our model" too
jargony for an explanation. Talk about setting data instead.

Re: Validation -- make sure that this section is up to date with the latest
version of Backbone before publishing, as this has changed recently.

"The events attribute" section could use a bit more meat. It's pretty core to
the View. An important thing to emphasize is that the declarative, delegated
jQuery events means that you don't have to worry about whether a particular
element has been rendered to the DOM yet or not. Usually with jQuery you have to
worry about "presence or absence in the DOM" all the time when binding events.

TodosCollection is an awkward name for it. Perhaps just call it Todos?

"Note that using Collection.reset() doesn’t fire any add or remove events.
A reset event is fired instead as shown in example." It's worth explaining that
the reason you might want to use this is to perform super-optimized rendering
in extreme cases where individual events are too expensive.

"The complete list of what Underscore can do is beyond the scope of this book,
but can be found in its official docs." I think this is a mistake. A huge part
of the usefulness of Backbone is the data-manipulation and aggregation methods
you have available to your models through the use of Underscore methods ... I'd
go over map, reduce, filter, any, all, groupBy, and probably a couple
of others at least. They make for great examples/demos.

Events

When first discussing events, it's worth mentioning what they are -- a basic
inversion of control, where instead of having a function call another function
by name, the second function registers its interest in being called whenever the
first named "event" happens. The part of the app that has to know how to call
the other part of the app has been inverted. This is the core thing that makes
it possible for your business logic not to have to "know about" how your user
interface works.

"since removing a View without unbinding its handlers can result in memory
leaks and other application bugs" Way too vague. Be specific about how if you
remove views at the same time their corresponding models are also removed, there
are no problems. It's only when the model sticks around and continues to call
a View's event handler that you've forgotten to remove that you might be concerned.

Routers

"In Backbone, routers help manage application state and connect URLs to
application events." Nope, they don't "help manage application state". All that
routers do is provide a way for you to connect URLs (either hash fragments, or
real) to parts of your application. Any piece of your application that you want
to be bookmarkable, shareable, and back-button-able, needs a URL.

"As of Backbone 0.5+, it’s possible to opt-in for HTML5 pushState support" These
days, just say that it's supported. And that it's vastly preferred if you're
capable of also supporting it on the server side ... although it is a bit more
difficult to implement.

"It is also possible for Router.navigate() to trigger the route as well
as update the URL fragment." Talk about how the first is the preferred form,
like dropping a bookmark when your application transitions to a certain place.
navigate: true is available, but discouraged.

Todos

At this stage in the game, it's a little unfortunate that this book uses the
Todo list app as its first example ... as I'm sure most folks are sick to death
of it (I certainly am) ... but that's probably the way it's got to be.

The repetition of large blocks of the same source code is a bit strange.

Exercise 2: Book Library - Your first RESTful Backbone.js app

Ahh, a non-Todo example. Lovely!

Nice work walking through the basic connection over to Node and Mongo. Folks
are going to love this bit.

    book.title = request.body.title;
    book.author = request.body.author;
    book.releaseDate = request.body.releaseDate;
    book.keywords = request.body.keywords;

Maybe use an Underscore function or two to simplify repetitive stuff like this?

"we will use the dateFormat jQuery plugin" Might be worth just using a couple
native JS date functions instead of reaching for another plugin for this.

"Another, simpler way of making Backbone recognize _id as its unique identifier
is to set the idAttribute of the model to _id." I'd just use this for your example,
using parse for this purpose is a bit silly here.

The addBook function looks kinda scary and manual. Instead of looping through
the inputs and switching like that, I'd try to clean it up with specific queries.
Also, currently using each and push instead of a simple reduce, and stuff
like that.

Backbone Boilerplate And Grunt-BBB

Just my opinion, but I'd strongly recommend removing this section. I think that
starting folks out with Boilerplate + Grunt is way more complexity than
beginners should have, and gets them started on the wrong foot.

Nesting

I wouldn't start by showing setElement -- that's just a way for folks to get
bitten. Start with append.

"Backbone.js doesn’t implicitly handle relations or nesting, meaning it’s up to
us to ensure models have a knowledge of each other." ... just like you would
in any other object-oriented piece of JavaScript. Objects can have arrays and
dictionaries of other objects that they refer to ... and those can be Backbone
Models or Views or whatever.

"One approach is to make sure each child model has a parent attribute" ...
probably only useful if you're actually modeling a tree.

Is it possible to have one Backbone.js View trigger updates in other Views?

Instead of starting with a mediator, start with just having the one view reach
over and trigger the event in the other view. It's usually a fine way to go --
especially if the event in question only serves to bind these particular views
together -- in that case, it's equivalent.

How would one render a Parent View from one of its Children?

Also a bit strange advice. The simplest thing to start with would just be:

this.parentView.render();

... only use events when an actual inversion of control is desired (which it may
be in this case, but if it is, that should be mentioned).

How do you cleanly dispose Views to avoid memory leaks?

The example BaseView is a pretty bad idea. Having a view keep an array of
bindings is a good way to cause memory leaks, not avoid them. Remove this
section and just use listenTo instead.

And, optionally, explain how references and GC works in JavaScript ... while
avoiding FUD ;)

What’s the best way to combine or append Views to each other?

The best way to combine or append Views together certainly shouldn't start with
Marionette. The best way to combine views is simple:

this.$('.inner-view-container').append(innerView.el);

... just plain jQuery.

Better Model Property Validation

The best way to do this probably isn't to stick validation in your model
attributes ... it's to have a function specifically designed for validating
that particular form. And there are lots of good JS form validation libraries
out there. If you want to stick it on your model, just make it a class function:

Book.validate = function(formElement) {
  ...
};

Inheritance

"If this sounds familiar, it’s because extend is an Underscore.js utility,
although Backbone itself does a lot more with this." Nope. Unfortunately,
Backbone.extend and _.extend are entirely unrelated functions.

Backbone Extensions

I'm not going to have many opinions on these, but perhaps a few...

You can probably remove the "Memory Management" section in Marionette, now that
listenTo just works out of the box.

"It’s an AMD-compatible version of the Underscore templating system that also
includes support for optimization (pre-compiled templates) which can lead to
better performance and no evals." -- Underscore supports pre-compiled templates,
which means no evals ... and it also supports the varname option, which means
no with statement.

It's a little strange to have the simple mediator source code included here,
given that you can explain how to use Backbone.Events for this, without having
to have a new (incomplete) implementation.

Looks like the section on Thorax got mis-copy-pasted to the bottom as well.

Conclusions

Deserves a little more time and love. I'd expand the conclusion by a page or two,
and try to really bring the book to a close here.

Owner

addyosmani commented Mar 22, 2013

Areas here requiring further work: conclusions, prelude, why consider Backbone.

@dcmaf dcmaf referenced this issue Mar 29, 2013

Merged

Update to Prelude #389

Owner

addyosmani commented Mar 29, 2013

@dcmaf I'm trying to decide what we should do about the section discussing the Front Controller pattern in MVC applied to the web. With the updates to mention Rails MVC in place, should we:

(a) Keep the Front Controller section as is - did you find it helpful?
(b) Move the Front Controller section to the appendix, but leave a paragraph in teasing it with a link to read more. Maybe the one starting "Other server-side implementations of MVC (such as the PHP"?

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - updating introduction, prelude a99243a

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342: rewritten why choose backbone section fca756e

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - some improvements, adding in nested models/collections piece 1b63a0d

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - simplifying what does mvc give us summary f07e84b

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - addressing further feedback 5663503

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - first take at improving mvc applied to the web a5c1faf

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - lots of updates to the mvc in the browser/client-side mvc …
…section
d75bbe0

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - updates model paragraph ae3ebf3

@raDiesle raDiesle added a commit to raDiesle/backbone-fundamentals that referenced this issue Mar 29, 2013

@addyosmani @raDiesle addyosmani + raDiesle For #342 - updates validation in internals 9eb9fbc
Owner

addyosmani commented Mar 30, 2013

Reviewing the Front Controller section I believe there's value in keeping it there. Closing this issue. Hopefully we've captured enough of what Jeremy was after!

@addyosmani addyosmani closed this Mar 30, 2013

Owner

addyosmani commented Mar 30, 2013

Thanks @raDiesle!

Collaborator

dcmaf commented Mar 31, 2013

I just read back through the Fundamentals chapter and I think the Front Controller section looks okay. To me, coming to client-side SPA development from a server MVC background (Java/Spring MVC in my case), this is easier to follow than the Ruby on Rails example.

I think the Ruby on Rails discussion could use a diagram similar to that in the Front Controller section. The Backbone sections (Client-Side MVC & Single Page Apps, Client-Side MVC - Backbone Style, or Implementation Specifics) could use a analogous diagram showing how client-side MVC works. I think something along those lines would help with contrasting client-side MVC with server-side MVC, which seems to be the intent of this section.

Collaborator

dcmaf commented Mar 31, 2013

@addyosmani It doesn't look like this change regarding Router.navigate() was incorporated:

"It is also possible for Router.navigate() to trigger the route as well
as update the URL fragment." Talk about how the first is the preferred form,
like dropping a bookmark when your application transitions to a certain place.
navigate: true is available, but discouraged.

Collaborator

dcmaf commented Mar 31, 2013

I find the updated solution to Better Model Property Validation (based on Jeremy's suggestion) to be confusing. It seems to be saying that the best solution is to not use the built-in Model validation support and to write an external validator instead. It then goes on to present a solution using the internal validate() function.

It might also be worthwhile to stick in a mention of the Backbone.Validation plugin and its preValidate method. I was using an approach along the lines of what is presented in this section and found using preValidate very helpful.

Collaborator

dcmaf commented Mar 31, 2013

I've finished reviewing the updates related to Jeremy's review. The new expanded conclusion looks good.

One thing I've noted is that the book consistently advocates using Backbone vs. a roll-your-own solution, but does not directly contrast it against other MV* solutions such as Angular and Knockout (although there are allusions along the lines of how it is minimal, etc.). It might be helpful (perhaps at the end of the fundamentals chapter) to discuss the framework approaches taken by other MV* implementations and how Backbone fits into the overall MV* landscape.

Owner

addyosmani commented Mar 31, 2013

@dcmaf Thanks for the feedback. I've added a note about Router.navigate and a few paragraphs on the validation plugin. To try making sense of the better model validation section, I've given the two plugins we discuss their own headers and moved Jeremy's advice to its own header as the final point in the section. That (I think) allows it to be presented as an alternative, rather than contradicting with the rest of the advice in the section.

Does it make more sense in it's current form?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment