Hacker News new | past | comments | ask | show | jobs | submit login

It looks like the meat of $.observable is mostly just passing thru to jQuery https://github.com/moot/riotjs/blob/master/riot.js#L29-L54 I don't really have an opinion on whether this is good or bad but it's worth noting that jQuery is doing the heavy lifting here.

A couple code notes:

* why, in the observable function, are you switching functionality not by the name of the function called ('on','emit', etc.) but by the index of that function name in the array on line 32? This strikes me as brittle because adding to or rearranging that array would cause the whole function to fall apart. It's also harder to read- I'm looking at `if(i == 2)` rather than `if( fnName === 'emit' )`. Does this yield a significant performance gain or is it just to juice the final "weight" of the library (<1kb)? I ask because it sure as heck makes the code harder to read.

* Using `name` & `names` as variable names makes the code harder to read, as it doesn't give any meaningful context about what the variable stores (besides the fact that it probably represents a thing that has a "name"). It looks like in this case it represents event names but more descriptive variable names would eliminate this ambiguity & it would make no difference to the code weight after minification.

I bring these things up because this lib looks cool & I like the "you can easily read the whole source" point, but this obsession with brevity (that's what it looks like at least) causes code readability to suffer & greatly diminishes the benefit of being able to "easily read the whole source." Make the code more readable & more people will use the project & contribute. :)




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: