Friday, June 12, 2015

6/12/15 - Confused on updating interfaces

I've been running into a problem lately: when I go to refactor one object's interface, I change some code, fix any failing tests, and then run the app. Unfortunately though, the way LibriFox is set up, it is very easy to have passing tests in an app that doesn't work. Each isolated test will work, but testing in isolation can be problematic when the tests rely on stubs of other objects that are no longer consistent with the actual app.

 For example, here's something I just ran into: All my tests pass, but since I know that passing tests don't equal a working app, I also go in and actually test the user story, to make sure it all functions correctly.  When I try to download a chapter, the app fails saying that the storage manager is undefined.  Looking at my code, I noticed that I had moved around the order things were defined in, so storageManager was being passed in as undefined.  This was an easy fix for me, but I wish my test suite could catch these errors for me.  Is there any way to do this?

Yesterday, I was also doing some refactoring and ran into a similar issue.  My tests were passing, but I realized that some of the tests were using stubs of other objects that were way out of date.  My other question is: is there any way to get a warning when your stub for an object no longer matches the actual object?  I'm assuming not.

I guess this is why user story testing is so difficult.

Thursday, June 11, 2015

6/11/15 - Storing and loading chapter data, and jQuery Mobile events

The app now writes JSON metadata to localStorage when the user downloads a chapter. This is pretty darn neat:
Index.html: no chapters have been stored locally yet Downloading a chapter of ABC Vegetable Gardening Index.html: saved book shows up After clicking on the listing, this screen shows tapping the chapter name will (temporarily) show the value stored in the path attribute.


I found this stackoverflow post about jQuery mobile page events that had good information on how the pagecreate, pageinit, and pageshow events can be used in the app. I didn't realize that pagecreate only fires on the initial page load, and I was running into issues where the index page wouldn't update the downloaded books list with books downloaded during that session. I moved my logic for populating the list element from a pagecreate event to a pageshow event, and now it works properly.

EDIT: something weird is going on, I'm not sure how correct that stackoverflow answer was.  The app is firing the pagecreate event multiple times as seen here (also, that not well-formed, fun stuff):


EDIT 2: w3schools also says that the pagecreate event should only be fired once, so it's possible that maybe these pages aren't caching normally due to our configuration. I'll have to do some more research on this. The reason why I care is because I'd like to have pages be cached in one direction, so that when a user goes back a page, it loads the cached version, but when a user goes forwards a page, a fresh version is generated.

EDIT 3: Adding the data-dom-cache="true" attribute to the chapters list <div data-role="page"> causes the chapters list oncreate to be called just once, but as a result of this, the chapters don't update after you click a book for the first time, because the page has already been created. I need some sort of middle ground between these two options.

The code that powers the user interface shown in these pictures doesn't have any tests written for it, and really needs a clean up (hard coded selector strings, somewhat duplicated code). However, testing these user interface events is going to be a huge pain, so I need to ask around and see if it's even worth testing.

Also, something interesting I found out just now - in order to get the images to go like that I had to do some html and css stuff, and blogger kept breaking it. I tried to figure out why so I ran my html through a validator, and it turns out that you can't put a div inside a span. I didn't realize there were limitations to where you could put divs!

Wednesday, June 10, 2015

6/10/15 - UI Testing in LibriFox

In refactoring LibriFox's search functionality, I moved the "pagecreate" event for the search page into a function within SearchResultsPageGenerator.  Now that it's inside a function, I feel like I should write tests for it (I mean, I should have written tests for it in the first place, but that's a different problem).  However, I would end up writing so much boilerplate in order to simulate the ui environment of the app, that I don't really think it would be worth it to write a test.  I would probably end up spending a really long time getting the test to work properly, even though the actual function would probably change very little.  When something is hard to test, it generally means that the object being tested is too closely coupled to its dependencies, but in this case I don't see how I can easily decouple it from the user interface itself.

I am running into a similar problem in trying to test this function, which enumerates over files in the filesystem.  This method depends on the interface provided by the browser, and so if I wanted to write a test, I would need to mock the enumerate, onsuccess and onerror functions as well as this#continue and this#result.  It's just a lot of work for something that (seems) to work just fine.

I think the problem lies in testing the objects that rely on external libraries or APIs.  My code is forced to interact with these in a certain way, so my tests have to work around that. For now, I'll just leave these two examples untested.

Tuesday, June 9, 2015

6/9/15 - NotesDemo Talk Reflection

On Saturday, June 6th, Chris and I ran a Firefox OS app development workshop at the National Day of Civic Hacking.  It was a great learning experience, both for the audience and for me as a presenter.  I found that some things I did worked really well, and some didn't work as well.

Things that worked:

  • Having a google doc that outlined what each step would do was definitely good.  It was a 'hub' for people to look at our materials, since all the github commit steps were linked there as well.
  • Having additional visual aid for some of the important/confusing aspects of the project.  These helped me articulate what I was doing in the code.  The directory of all the materials, including visual aid, is here.
  • Walking around and helping people out when they had questions was a valuable experience helped the audience members get more acquainted with the code, and helped me get a deeper understanding of where things were likely to go wrong.  More on that in the things that didn't work section!
  • Showing github diffs helped show people what files should be modified.  I just need to make sure I explain what they mean first, because one audience member copied a diff literally and ended up with both the removed code and the new code side by side.  Some people also copied and pasted from the diff instead of from the actual file, and ended up with plus signs at the beginning of all their lines of code.

Things that didn't work

  •  Trying to explain closures and closure scope was difficult, just because the audience didn't have quite the CS understanding needed to see how closures work.  It also didn't help that I named the function I was demoing 'scoped' in my closure presentation, so it sounded weird when I talked about the scope of scoped, which threw me off.  In the future, I would want to go with a more distinct name.
  • Too complex of an app - we ended up having 10 different files for the audience to add and work with throughout the duration of the talk.  We actually had to skip over a couple of the script files and treat them as 'invisible magic', just so we could finish in the time given.  A lot of the concepts the app introduced, like functions being closures and functions returning functions, while important patterns to be aware of and understand, end up being lost on an audience that doesn't have much experience with javascript in the first place.  It would be more beneficial to focus on the basics instead.
  • Having all the steps up on github at all times allowed people who got bored to jump ahead rather than taking their time to try to understand the code.  That, along with people copying and pasting rather than typing it for themselves, let people completely bypass a lot of of the learning they would have gotten by actually working through the code.  This also left people at far different stages of development: the people who were actually typing line by line were much earlier on in development than those who copied and pasted, meaning that it was harder to answer questions because people might be on different steps entirely

Things to do for next time

  • Simpler app with more of a focus on the layout - I realized that it's probably more important for people interested in web app development to learn the html and css skills first, before focusing on the javascript.  With this in mind, if I workshopped a second app, I would make it more html heavy, with less emphasis on the javascript.  NotesDemo only had a barebones index.html page, with a lot of the html being created inside the javascript. 
  • Fewer files to work with - NotesDemo ended up having a bunch of different js files that all depended on each other, so it was hard to go step by step and have a working app at each stage.  The app didn't end up visually doing anything until step 5, because the view generator had so many other dependencies.  This goes hand in hand with doing a simpler concept next time.
  •  Try to avoid plural names - a lot of people missed the 's' in 'notes_ui_manager', or added an s onto the 'note' in add_note_ui, which is a hard mistake to debug if you don't know what you're looking for.
  • If at all possible, only push the next step to the repo when everyone is ready for it.  Encourage those who are done to help those who are still working.
Overall, it was a great experience, and I think everyone involved got something out of it! I certainly did.

Monday, June 8, 2015

6/8/15 - NotesDemo oops

Some of the hooligans in the Firefox OS workshop realized that I didn't properly sanitize the inputs for my note title and body elements, which is pretty bad.  The problematic lines are here; I'm just concatenating strings and user input to create html elements, which is BAD.  If you go to the notesdemo page here and create a new note with the title or body as
<script>
$('<div class="hax" style="height: 100px; background-color: red; color: white">THIS PAGE HAS BEEN HACKED BY 4CHIN</div>').prependTo($('body'));
setInterval((function () { var i = 0, arr = ['red', 'blue']; 
  return function () { $('.hax').css('background-color', arr[i % 2]); i++; };
})(), 200);</script>
it will do something silly. This code is safe, but in general don't go executing random javascript code in your browser...

This 'hack' will survive page refreshes, because it is being saved and loaded from localstorage.  It's not a huge security vulnerability (as far as I can tell) though, because only the user (and the scripts already on the page) can add notes and access localStorage, and notes are not shared across browsers.  Still, it's something that should be fixed.

6/8/15 - LibriFox Not Well Formed

Now that I don't have to work on NotesDemo anymore (the talk went well, I'll try to write a reflection post on it), I'm back to LibriFox. I did some research on the not well formed error we get every time a new page is loaded. I'm not 100% sure, but it seems like this problem is related to another problem that pops up in certain cases: mismatched tag. Expected: </link>. A link tag with no closing is 100% valid html, whereas including a </link> tag would actually be invalid html.

These two errors are both related to how jQuery mobile loads new pages. It uses an ajax request to dynamically load the body of the requested page into the original page. Interestingly enough, it doesn't load anything in the <head> of the new page, because it assumes that it will be loading the same scripts that have already been included in the original <head>, and so it just ignores the new one completely.

Because we are loading ajax from the local file system instead of from a server, it seems like the problem is that the local files don't have the proper HTML headers, and so the loader is treating them like x(h)thml, even though all the pages have a <!DOCTYPE html> as the first line. This is very annoying, but it doesn't seem like there's anything I can really do about it without hosting the app on a webserver.

Monday, June 1, 2015

6/1/15 - Removing init functions

After writing my previous blog post, I tested require.js and found that requiring the same file multiple times does in fact give you the same instance. Since this is the case, I no longer need to pass the storage manager into functions returned to construct disp_note_ui and notes_ui_manager. Since these functions now take no arguments, they don't need to be functions at all, and the objects can be returned directly. I removed the _init suffix from these js files.

For example, in generate_notes_view.js, instead of notes_storage_mgr being defined by being passed into this function, it is just passed in by require.js here.

I did this in a new branch; I'm going to hold off on merging this to master until I talk to Kevin. Personally, I think it's an improvement, but I'd like to get another opinion first.

6/1/15 NotesDemo mega post

Over the past week I've been working on completing NotesDemo, the app I'll be presenting at this year's National Day of Civic Hacking in Alexandria. A lot has changed since I last blogged about it. Here's a screenshot of the completed app:


First, I'll talk about the process I had to go through in getting my existing code to work with require.js. This pull request contains the commits involved with adding require.js to the app. In index.html, instead of setting the src a script tag to my js file, I put require.js in the src attribute with an additional data attribute that points to my main js file.

I had already been using the factory pattern (My code was slightly different than that example, though), where you would call functions that would return the object you wanted. Require.js seems to be set up with this pattern in mind, so it was incredibly easy to convert my functions over.

For example, this function became this file once require.js was added. All I did was surround the function with a define() function, which is require.js's method for defining modules (I think they're called modules, I want to try to get the terminology right for the talk). Then, when I want to go and use my storage manager, I import it and assign it to the notes_storage value (the order of the imports is the order of the arguments for the passed into require(). Then I can use it just like any other variable!

However, this way of doing things doesn't allow for any arguments to be passed in to the define() statement when the object is being created, so if there are other dependencies, a different approach must be used. Instead of directly returning the object, a function is returned with any arguments that will be used in the creation of the object. An example of this is here, where the generate_notes_view object needs the storage manager instance.

I added an init suffix to all the js files that return a function rather than an object. I might be able to get around needing a function to pass in notes_storage_manager at all if I had added 'app/notes_storage_manager' to the dependencies in the define statements of all the script files where it was needed. However, I felt like that would give me a new storage manager instance each time, while I want only one instance across the whole app. I'll have to do some experimentation with that when I get the chance.

EDIT it looks like multiple imports of the same dependency share instances, which I guess makes sense. I tested this by creating a js file dependency_module that generated a random number as its argument. Two test files then included dependency_module as a dependency in their require statements. This is the output I got:
"creating dependency with rnd property 0.37818958461331087"dependency_module.js:3:4
"1: got object with rnd property 0.37818958461331087" test_file_1.js:2:4
"2: got object with rnd property 0.37818958461331087" test_file_2.js:2:4
"object1 equals object2:" true app.js:20:4
One thing I really disliked about require.js was the jQuery boilerplate I had to write in each file that needs the library. In order for jQuery to work with require.js, first you have to alias your jquery-x.x.x.min.js file to just jquery in your require config. This fixes some issue with jQuery defining its global variables. Next, in each file that requires it, you have to add the 'jquery' dependency and map it to the $ variable, like so.


Design Questions

Now that I've completed it, I have lingering questions about some of the design specifics. For example, I have this object notes_ui_manager that generates add_note_ui and notes_view objects and exposes them through its interface. However, the only reason I need to expose them at all is because I'm arbitrarily doing some work in my $(document).ready() that could really be moved elsewhere. If I do move this code, I can get rid of the public interface for notes_ui_manager, but that makes everything even more opaque, so if I did ever need to access the note_add_input object outside of the ui manager for example, it would require rewriting code. Would moving this code out of the $(document).ready be better than leaving it in?

Another problem I see with this app is that most of my code is just objects creating other objects as private variables: the only thing actually passed around at all is the notes_storage_manager; everything else is enclosed. For example, the edit_note_ui object is instantiated inside the generate_notes_view object, which is instantiated inside the notes_ui_manager object, which is created inside app.js. At no point is edit_note_ui ever passed in as an argument, which would make testing the classes that rely on it very difficult if I chose to add tests. Is this something I should be worried about?

Some of my objects only have one method in their public interface, like this one. Since functions are already passed around all the time in javascript, would it make more sense just to return the function directly rather than returning a single-function object?