Sunday, August 9, 2015

Javascript app design - More questions

Kevin responded to my blog post with some comments and suggestions:

Args parameter

Kevin mentioned that using an args parameter when instantiating an object is 'opaque'; it's hard for a programmer to know what the actual dependencies of that object.

I agree, the args parameter does make it more difficult to figure out what the dependencies of an object actually are. Often times when refactoring an object I would forget to remove something from the code that pulls objects out of the args hash for use, and end up with a misleading var statement (for example).

When using and refactoring these objects, the instantiation code in #createApp() was how I knew "what went where", which sometimes caused issues when the stuff being passed in via the args parameter no longer reflected what the object expected.

I chose to use the args parameter because you generally want to avoid forcing the caller of a function to have to know the argument order. I guess for objects with fewer arguments (less than four?) it might make sense to just use regular calling structure.

Combo getters/setters

Kevin advised against combo getters/setters, and linked me to this page for explanation.

I assume that combo getter/setter means these types of functions, where I could either call Player#position() to get the current position, or Player#position(number) to set the position.  I agree that using a combo does make the code uglier. I was sort of experimenting with using them, as everything in jQuery and vanilla javascript follows this combo pattern. For example, in vanilla javascript, AudioElement#currentTime gets the current player position, while AudioElement#currentTime = number sets the current time.  My Player object is essentially a more feature full wrapper for AudioElement, although I suppose it doesn't really matter whether I match their interface or not.

#registerEvents functions and selectors

A big question I had is whether I should keep the call format of my #registerEvents functions consistent.  In most of the objects' #registerEvents functions, an object hash of css selectors is passed in. However, in the SearchedBookPageGenerator object, it was easier for me to just have the selectors passed in on instantiation since (I guess, it's been a while since I wrote this one) I didn't want to have to pass the selectors object through all the different functions.  The call to this ends up looking kind of weird though, it doesn't fit well with the others and could be misleading, especially if a programmer does erroneously pass selectors in with #registerEvents, which will cause no errors and simply ignore the selectors.

I also ran into the problem of needing selectors all through an object with StoredBooksListPageGenerator, but I took a different approach that also isn't so elegant. Would it be better to just have selectors passed in to each private method in this object? It would certainly be more explicit that way.

Test Coverage

How important is total test coverage? There are certain parts of the app that are very hard to test, and I feel like it would be a waste of time to do so. For example, most of the PageGenerator objects aren't tested, because by their nature, the things they do are very visual and user facing. I could try and test them, but it seems more natural just to go to all the screens on the app and ensure they work properly than to try to write tests.  The same is true for objects that act as a wrapper(?) for the Mozilla apis, such as my FileManager object.  I suppose I could use spies to test that the filesystem functions were called as expected, but again, is this worth doing?

Thursday, August 6, 2015

Javascript app design - final questions

As my development of LibriFox is coming to a close, I still have some lingering design questions about javascript development in general.

Indentation

One concern I have is with indentation: due to the functional nature of the language, there is a lot of passing functions into other functions, and sometimes it is hard to tell where the indents should go.

I especially have this problem when dynamically creating html via jQuery. For example, creating a book list item: Note the ugly 8 space indentation jump between the last two lines.

I thought about this some more, and tried a slightly different formatting approach, which fixes the gap between the last two braces. However, this style is also more inconsistent, because the method calls are sometimes separated by line breaks, and sometimes not. There must be a clean way to do this that I'm just not thinking of!

Object Instantiation

A more serious problem than just indentation, I think, is my app instantiation code. I tried hard to follow best practices, but the code I ended up with to make all my objects work together is really, really ugly. LibriFox is the most complex project I've worked on to date, and I don't have any experience on instantiation patterns for so many objects. I ended up with 110+ lines of just creating and setting up objects.

It might make sense to group these objects based on their purpose, and make an object for each group (so all the page generators would go into one group, all the objects dealing with downloading chapters in another, etc.). However, this also seems problematic, because some objects might not fit cleanly into whatever category I define for them.

File Length

Another problem I see in the app is file length. The main javascript file defines all the app's behavior, and as a result is currently 2,149 lines long. Javascript has no built in module system, and I never got around to converting the code over to use something like requireJS.

I am using the mozilla LazyLoader library already to load the MediaDB and asyncStorage code, so I may want to split app.js into smaller files, and then load them using that. However, that may slow down the app load due to reading from multiple files. It will also necessitate a HUGE array of filenames inside the LazyLoader#load function, as I don't believe the library supports filename expansion using the * wildcard. I could always add this functionality in, though.