Tuesday, March 17, 2015

3/17/15 - Good Enough?

I realized as I was blogging yesterday that our Book object was beginning to take on too much responsibility, as we added a currentChapter property to manage which chapter the user had clicked on in the interface. We definitely don't want Book to know anything about the actual UI state of the app, because that is well beyond it's scope of use. Instead, I added a UIState which takes the place of selectedBook as our global variable for passing book data into the pagecreate methods.

One thing I ran into was the problem of naming my methods. I've never been all that creative at coming up with names, and I feel like you have to find a balance between descriptive and succinct. For example, I wasn't sure whether to go with #setCurrentChapterByIndex() or #currentChapterByIndex() or something even shorter than that, which would look better, but be less descriptive about what the method is actually doing. Since the method is setting the current chapter based on the index, I opted for the former method name. I could remove these methods all together, but getting chapter and book objects by index/id seems to be a pretty common occurrence in our code, and I'd rather not duplicate code by scattering this conversion throughout our page generation methods.

Trying to have descriptive method and variable names is one of the reasons I prefer snake_case to camelCase: I try to write my method names like they're a sentence describing what the method does, which lends itself much better to snake_case, because the underscores, rather than capital letters, denote spaces. In this case, my method could be described by the statement 'set the current chapter by using the index', which would then become #set_current_chapter_by_index. I find this to be more readable than #setCurrentChapterByIndex, although that's just my opinion. Since the javascript standard is camelCase, I'll stick with that.

I'm also not sure whether these methods should return the values they find (lines 8 and 12), since they are supposed to just be setting the value within the object. Setter methods returning values is a very Java thing to do, and Java isn't that great, so maybe we should avoid that. Actually, now that I think about it, these return values aren't actually ever made use of when calling the methods, so I'll just remove them. I'm going to leave this part in my blog, though, because it's still a good thing to think about.

Here's my commit diff, and here's what I ended up with for the UIState function.

I also noticed that the <code> tag styling was sort of hard to read, so I darkened the font color and added some padding. Here's a comparison of the new code tag style and the old code tag style (notice the text doesn't go all the way to the edge of the gray rectangle anymore).

1 comment:

  1. I added a new story:

    https://trello.com/c/p0l20Zdx/13-remember-play-position-in-multiple-books

    that will likely have an impact on how you do this.

    ReplyDelete