3/16/15 - Refactoring with Friends!

Today Finn and I started refactoring together.  I think my code gave him a good example of how useful Javascript objects can be as a store of data, and I also explained some other stuff that had been bothering me about the code, like the idea that duplicated code should be extracted into a method rather than copied and pasted.

We ended up merging the alex_refactor branch with the master branch, which went without a hitch because no changes had been made to master since I forked it on Thursday. Today, we continued refactoring where I had left off from yesterday.  Here's a comparison of our code between today and yesterday (focus on app.js)

[Also, sidenote, I figured out how to compare snapshots of the repo at certain times, as seen in the comparison link above!  I used this resource to figure out how to set the times, and although I didn't use it, this resource is cool: comparing the repo between specific commits (which don't have to be right next to each other)]

We removed a bunch of comments that were no longer necessary and just took up lines, as well as this whole section of code, which was no longer necessary because of our Book persistence between methods.

We were also trying to figure out the best way to keep track of what book a user is currently interacting with.  After a user selects a book from the search, we need a way for the resulting chapter and book page generation methods to know which book was selected.  Previously I had been passing an id in with the url, but we opted instead to go with a global selectedBook variable, at least until we come up with a better solution. I realized passing the id in with each url didn't feel right, as we would have had to include the id in every single app url for every single page, when it makes more sense in my opinion just to have a global variable.

Here's what we ended up with. The book-id attribute of the search result is set, then retrieved on click where it is evaluated to a book object from the bookCache and set as selectedBook. I don't think this is a perfect solution, but it certainly seems less clunky than passing the id in via the url and having to parse it out again in each pagecreate.

I think it's also worth talking about a very minor thing Finn and I came across when working on getting the app to respond correctly to a user selecting a specific chapter to listen to. We need some way to store the specific chapter that was selected, and our current approach as of the end of class was to say that we'll set this value in Book once the user selects a chapter from the chapter selection screen. I wanted to make it clear that the Book object would sometimes have a currentChapter property defined at some point, so I had Finn add this line to show anyone who is unfamiliar with our code that this key might exist at some point. However, I don't know if that follows Javascript's style conventions.

Also, now that I'm looking at it, it seems like our Book object is taking on too much responsibility: it's acting as both a store of book data and as a store of UI state, which is BAD! I think we'll need some separate construct to store the UI state so we don't end up with all these super-objects that know everything about everything.