joseph / Monocle

A silky, tactile browser-based ebook JavaScript library.
http://monocle.inventivelabs.com.au
MIT License
743 stars 200 forks source link

boolean cleanup #212

Closed danshultz closed 10 years ago

danshultz commented 10 years ago

I know we talked about your feelings of == vs === but I thought I would push this anyways for discussion.

While I have not personally hit bugs do to these comparisons, I do prefer (foo) and (!foo) vs (foo == false || foo == true). The biggest reason I prefer this is for clarity of what is acceptable in scenarios but then again, I also compare against length using (arr.length).

The second reason I'm pushing this commit is because I wanted to also start a discussion about including jshint to help ensure consistency, etc across the javascript.

While jshint is very customizable - one area that it doesn't really waver is comparisons of (== true) (== false) and (== 0) which to me makes sense. This may be dogmatic but I think it's more about clarity.

I would like to push a jshint file in another commit which will allow quickly 'linting' the js based on your preference and what styles I noted already present in the application.

Thoughts?

joseph commented 10 years ago

Hm, I think that in every case of a direct comparison to a boolean value in these files, I should have been using ===. I was specifically looking for false, so if (!foo), while an accurate translation, is a misreading of my intention.

joseph commented 10 years ago

Thanks, I brought these recommendations in with the adjustments I mentioned above.