Thursday, March 14, 2013

Coding for review


<ship, ship, ship!>

This past Friday, I shipped about a month's worth of extensive refactoring to the content editing infrastructure of the site. As with many refactoring projects, the best possible outcome would have been that no one would notice I'd done anything. The worst would have been visible breakage of the site for content authors or users. That launch was a success (whew!), and although it took a lot of effort and planning on my part to make that happen, I want to credit two powerful methodologies that ensured I wasn't working alone: unit tests and peer code reviews. I want to focus on code reviews here, because while the benefits of unit tests (especially in refactoring projects) are well-documented, I've found that subjecting code to peer review has subtly and unexpectedly changed the way I actually write my code.

The exercises team might be having a little too much fun on this code review.
<come all ye reviewers>

About a year ago, Khan Academy instituted a policy to peer review all non-trivial code commits. For coders who don't follow this regime, there are several benefits we were looking for:

  1. Fewer bugs. Bugs won't reach production if they are caught in a review first.
  2. Improved code quality. A strong check on coders' tendencies to take shortcuts, sacrifice code readability or understandability, or put in temporary half-measures that don't solve the underlying problem. In many cases criticism in a review caused me to rethink a problem and come up with a more elegant solution.
  3. On-boarding of new developers. Getting your head around a new codebase can be challenging. By reviewing new devs' commits we catch redundancy, unwanted side effects, and potential conflicts, as well as enforcing our style guide and setting a standard for high quality code from the very beginning.
  4. Diffusion of knowledge. Anything that facilitates communication between members of the team pays dividends over the long run. If nothing else, there will be a day when a critical developer will be on vacation/trapped in an elevator/at home with the swine flu and a reviewer will come to the rescue.
It's worth acknowledging the obvious cost to reviewing every commit: time. Time that used to be spent writing, testing and committing new code is now taken up with conversation and iteration on already-written code. So is the net result a drop in productivity for the entire development team? Not necessarily. Let's look at the list above again:

  1. Fewer bugs. A bug not caught during review will still have to be found and fixed later on. Tracking down a bug in production takes significantly more time, and fixing it is more difficult.
  2. Improved code quality. Quality code is easier to read, easier to implement new features on top of, and incurs less technical debt (TODO-laden code that will have to be revisited in the future). Of course, code reviews don't force this, they enforce whatever the team decides. If the team needs something done yesterday, then by all means do the quick thing and come back and fix it later.
  3. On-boarding of new developers. Efficient mentoring of new developers bends the productivity curve in a positive way. Even experienced devs are less productive while they are learning a new codebase.
  4. Diffusion of knowledge. Good documentation won't replace having multiple devs who understand any piece of code. Projects won't stall because the one developer who knows a system is busy doing something else and the code isn't clear. Our mantra is "anybody can fix anything", and it's critical that anyone be able to jump into any piece of code and understand it.

I won't go much further with this argument; in my mind it's a settled matter that compulsory code reviews are a Good Thing and they have helped us in many ways as an organization. What I hope to share here is a surprising and totally non-obvious fact: code reviews have changed the way I design and write code for the better.

It's actually night-time in this photo.
Such is the awesome power of teamwork at Khan Academy!
<evolution of a code monkey>

All the lessons I've drawn from this ongoing experiment have taken some time to understand and internalize. When we first instituted mandatory code reviews, I didn't notice any immediate changes. For trivial bug fixes, reviews are transparent: I make the fix, commit to stable, and send off the review after the fact. The fix might ship before the review is done, but bug fixes are high priority and that's OK.

Similarly for minor features: I make the change in a private branch, test and document, then send a review. Then there is a period of  answering reviewer questions and iteration. In the meantime I might move on to other work, and when the review is accepted I merge to stable and deploy.

With larger projects and changesets, I began to notice breakdowns happening when I got to the review stage. Reviews were too large for reviewers to comprehend, or too convoluted to follow. By the time a reviewer responded to a review, I would have several more reviews open for subsequent commits, and it wasn't clear which review fixes should be assigned to. Reviewers were reviewing already-replaced or rewritten code. It became a real mess.

While it was clear what the problem was - too many and too large reviews - the solution wasn't obvious. I could cut the size of the commits, and stop to wait for reviewers before proceeding, but that would mean dramatically slowing my progress - a busy reviewer can take hours or days to thoroughly read an important review. Instead, I adopted (with lots of guidance from coworkers +Ben Komalo, Craig Silverstein, and Ben Kamens) some habits that enable me to get useful feedback on code reviews and use that feedback effectively. Here is what I learned:

  • Make one conceptual change per commit. (As opposed to one functional change per commit.) When I started working on a change, I would often be thinking about a requirement: "The user can set a bookmark." I would add a UserBookmark object, write the get/set API calls, and some UI. Later I would come back and write some unit tests. This is all one functional change, but many conceptual changes, and they deserve their own commits: A new UserBookmark object, with full documentation and unit tests. Then the API calls, with their own documentation and unit tests. Each change is much easier to understand, can be critiqued on its own merits, and will tend to be confined to a particular part of the code.
  • Cut the thread. Many times, especially while refactoring, a change will have cascading effects: While testing I find multiple side effects from my change, fix them, and then those fixes cause more side effects, and so on. Sometimes fixing a side effect triggers another refactor, or fixing a totally unrelated bug, and when the code finally gets committed it is both unreasonably large and completely unfathomable. Even I can't remember exactly what prompted a particular change in a day-long marathon of bugfixing. I could try to split the fixes among several commits after the fact, but that's error-prone and difficult. A better solution is to "cut the thread" at some reasonable point, and start sprinkling TODO's liberally where fixes need to be made. It's clear to the reviewer that this code is not yet functional and just what the side effects of the change are without actually fixing those side effects in that commit. Best of all, it's clear from looking at the commit history what motivated each fix.
  • Write throw-away experimental code. When starting a project, I find it helpful to quickly iterate on a prototype implementation for a thorny code problem before settling on a final solution. These sketches are not useful to have reviewed; rather it is better to write up the proposed solution in a Google Doc or email and iterate on that with reviewers before sitting down to implement it for real. The second implementation takes into account reviewer feedback and is written more carefully, with documentation and unit tests that would be a waste of time during a prototyping phase. The prototype is eventually thrown away and doesn't become part of the commit history.
  • Work on multiple tasks in parallel. There is some inevitable downtime while waiting for reviewers to look at newly submitted code. Having a list of bugs or small tasks unrelated to my main project gives me something productive to do in that downtime. It's a great way to make sure that small, lower-priority tasks don't get crowded out by larger projects. I can assign the reviews for these smaller tasks to different reviewers, balancing the review load among the team.
  • Don't get too far ahead of the reviewer. I do my best never to push changes that build on unreviewed changes. When I can't switch to a different task, I keep new changes local and don't push them until they previous reviews are done. That way I can implement changes from reviews cleanly on top of the pushed commits and rebase my local changes on top. (I use Mercurial bookmarks or Git branches for this.) Sometimes after discussion with the reviewer changes will be made that force the later work to be rewritten, and that is fine. It's better than having to roll back later commits to fix something from a review!
  • Document everything, even if it's in progress. If I'm going to have to explain some complex bit of logic or a long list of method arguments to a reviewer, I may as well just do it in the code itself. Even if the code has ## TEMPORARY ## or // TODO(HACK) UGLY HACK all over it, it still gets documented. It never ceases to surprise me how long those things live in the codebase, and the short-term solutions are the ones that need the most explanation: Why do we need this? What should replace this? When can it safely be replaced?
I firmly believe that all of these techniques make my commit history easier for me and others to understand and make me more efficient as a programmer, and I probably wouldn't have adopted them if not for the practice of peer code reviews. I have also learned a lot about Python/JS/IE/life from my peers and maybe taught someone something they didn't know.

Even if you work alone or don't do code reviews in your team, perhaps you may benefit from these tips in your own work. If you do participate in code reviews, do you organize or think about your code differently to take full advantage of the review process? I'd love to hear from you.

< / tom >

Photo credits: Jason Rosoff, embedded Khan Academy documentarist and lead designer.

Tuesday, October 9, 2012

Going Back to Work


< birth >

About a month ago I had the great fortune of having an amazing new person stumble into my life, ready to challenge by his very existence the way I think about myself and my role in the world. That person is my first child, a beautiful baby boy. Even at his young age he is already full of contradictions: At times, he studies the world with the attentiveness and calm of a philosopher, taking it all in without pausing to divulge what pearls of wisdom he has gleaned. By contrast, he is barely cognizant of his own internal state and it's taken weeks of careful observation to discern which cries are hunger, gas or fatigue. We have had to teach him how to eat and how to sleep, which has really driven home the point that literally everything he will know or do as an adult decades from now he will have to somehow be taught.

Two weeks after he was born I returned to the office to resume work. There was a pang of regret that I expect many a father feels at being away from his only child all day, but there was also something else I didn't expect: a new urgency, a ticking clock that wasn't there before. I'll elaborate in a minute, but first some back story.

< childhood >

Last year I was a computer game developer. There is a fair amount of truth to what most people assume about game developers: we spend a respectable portion of our time play testing games, the atmosphere is one of creativity, fun, and self-expression (relative to most software development teams), and in the end we have our share of fans because we are essentially in the same industry as Hollywood and record labels. There is also however a fair amount of dysfunction: unrealistic deadlines, dysfunctional corporate partnerships and licensing agreements, and a disturbingly high rate of failed studios and canceled projects. I could write a whole blog post on this subject. It's a shame, because I know from experience that games can elicit strong emotions, games can change attitudes, and most importantly, games can teach. This last point is important because it got me thinking seriously about how I could use my technical skills to make an impact in the field of education.


The observation that depresses me most about our education system in America is how little things have changed since I was a child. Students I speak to going to elementary, middle and high school now have almost the same experience in class that I did - they are taught the same subjects in the same ways. So, what is the problem? Just look at the rest of their lives: they carry in their pockets computers orders of magnitude more powerful than those of two decades ago, social networking connects them in realtime with their friends and family, and millions of videos (at least half of which are not cat-related) are free to watch anytime. The possibilities for learning have never been greater: Wikipedia dwarfs any encyclopedia or public library, you can visit many museums and art galleries virtually, thousands of books can be downloaded wirelessly onto a single portable reader, and many research journals are now publicly available online. Such massive technological changes in the world have been incredibly disruptive in many industries - finance, commerce, entertainment, journalism, health care, art, the list goes on and on. But no matter how much we all agree that these same seismic changes will eventually happen in education, as a society we have so far been too timid to experiment with new ways to teach our children.

< maturity >

Enter Khan Academy. Sal's project began very humbly, tutoring his niece in Math over the Internet. I have also tutored middle and high school students in Math, and there is a key insight I took away from the experience: the students who needed tutoring were not dumb, or slow, or stupid. Surprisingly, the thing they all had in common was a serious gap in their understanding of some basic concept. Over time they had developed an attitude of defensiveness and unwillingness to address the issue, especially if the key concept was several grade levels beneath them. As a tutor I found myself in the awkward position of trying to get someone through Algebra who didn't really understand the relationship between fractions, percentages, and decimals and who felt that understanding to be permanently out of her reach. What I saw Khan Academy doing was addressing both issues: By using videos, a student who needs basic concepts explained over and over can get that in the privacy of their room without feeling judged or rushed. And by encouraging self-paced learning, a student will not be tutored in Algebra until she has demonstrated a thorough understanding of the prerequisite concepts. Most importantly, I saw no other company that was experimenting with new teaching methods and reaching actual students. All these reasons convinced me that by joining this team I would have a positive social impact, and that is what I did.


By its very mandate, Khan Academy is disruptive. We put resources and tools out there for free, for anyone to use, and let schools who dare to experiment with new ideas and teaching methods come to us. The fact that schools and districts have actually done so is very encouraging to me, and it gives me hope that by the time my son reaches school age, schools all across the country will be experiencing a much-needed renaissance of creativity, experimentation, and progress. As a parent I want to encourage him to find his own interests, his own voice, and a love for learning for its own sake. Rather than being a conduit for dry instruction, schools can teach kids to be informed citizens and morally conscious people. This is far more important and difficult than conveying a specific bit of knowledge or administering a standardized test. Sure, it is counterintuitive to speak of online education as a key tool for unlocking the human element of education, but that is what I see in the pilot schools and in our community of students and teachers. There is a hunger for change, and as more startups appear to feed that hunger the change will accelerate.

< parenthood >

The clock has been ticking for a long time, even though it didn't hit home for me until my son was born. It lights a fire beneath me to work harder at this job that I love, not just because of the wonderful people I get to work with but because we're working toward a common mission: to make education accessible and effective for everybody. This is not an abstract goal - in the next decade tens of millions of children will walk into their first classroom. How many will eventually go to college? How many will be able to compete in a global economy? How many will develop a lifelong love of learning?


As much as I would love to spend all my time staying home with my son, I take comfort in the knowledge that what we strive for every day is a brighter future for him and millions like him all over the world. I couldn't imagine a more rewarding way to spend my days.

< / tom >

P.S. Do you write code? Do you get goosebumps pondering the potential of technology to transform education and get a whole generation excited about learning? Do you enjoy freshly baked bread? We have a software developer position open! Email me at tom at khanacademy.org to find out more.

Wednesday, May 2, 2012

Progress vs. the Law of Useless Numbers

Statistics: The science of producing unreliable facts from reliable figures.
-Evan Esar

There is a trendy practice being advocated in a number of software development teams, which is to measure everything and A/B test radical ideas. It sounds good on paper: Instead of crowding together in a meeting room and debating pros and cons or relying on a designer to make all the decisions, why not let the customers vote with their clicks? On the other hand, there are some outspoken critics of A/B testing who claim that it marginalizes good design and it leads to bad decision-making in many cases. At Khan Academy, a company that provides a free service to a global population diverse in age and nationality, there is really no way to know what the effect of many of our changes will be except to try them and measure the results.

The result of our experiments thus far boils down to this:

Data is easy to collect but hard to interpret, never giving you a clear result that confirms your hypotheses. However, even noisy and confusing data is invaluable in forming and testing hypotheses about user behavior.

Long story short, so far the best we've gotten is some comfort knowing we haven't made things worse, and maybe a little insight into how users behave contrary to our expectations.

< process >

Last week at Khan Academy we completed a sequence of experiments over a few months aimed at improving navigation through the site. On this project we intentionally divided the work into small shippable changes so we could observe the changes in user behavior. Why is this process important? Because we as developers are biased by our prior knowledge of how the site works and how we want it to work, and the best direct user feedback we can hope for is a vocal minority. We will tend to err on the side of changing everything, and the vocal minority tend to be those highly invested in the status quo. Therefore, we need objective measurements of how visitors to the site behave and how this changes over time.

Our data collection is as comprehensive as possible: we report user actions to Google Analytics and MixPanel. We also collect conversion data for A/B tests using GAE/Bingo. After each deploy, we monitored all these statistics for a week to see the effect of the changes on both weekend and weekday traffic, which can differ dramatically. Then we took the results into account when deciding the next steps.

< long list of links >

The khanacademy.org homepage is a textbook example of a KISS solution that doesn't scale especially well. When you click the "Watch" link you get this:


(It is still here, if you want to exercise your scroll wheel)

Rather than diving in and trying to improve things, we started by trying to understand user behavior. How effective was this layout (long page full of text links with a sticky navigation header at the top of the screen) at getting users to the video content they are looking for? Here is some sample data from MixPanel:


What you see here:
  • Of all the users in the sample who landed on the homepage, 22.29% selected a topic from the topic browser. (There are other ways to get to videos, but it's useful to know what subset of the population we're looking at)
  • Of those who clicked on the topic browser, 80.67% clicked on a video in the Big List of Links.
  • Of those who clicked on a video, 86.71% started watching the video.
  • Of those who started watching a video, 74.27% completed it.
There is clearly a lot of room for improvement here.

< the fun part >

So now we come to the changes we've been rolling out. Since I last wrote about our topics reorganization in February, there have been two major changes tested and shipped that take advantage of the new structure:

The Watch Menu
The Algebra Topic Page

The Watch menu replaced the link that takes you to the top of the video list with a drop-down menu of topics that initially navigated you to the specific topic you selected, and eventually to the relevant topic page once those were released.

Topic pages were designed carefully to help visitors find content faster without overwhelming them with thousands of video titles. They are also a big step forward visually - I am particularly proud of the Art History page. Go on, have a look. Isn't it beautiful?

Aside from helping visitors get their bearings, topic pages afford many secondary benefits: They are easy to bookmark and link to, they can load much faster than the homepage, they can be included in site search results, and they can be indexed directly by search engines. As we build new features based on topics, they will naturally be surfaced here, and doing further A/B tests on these pages is easier than doing them on the homepage.

< the results >

Here is a chart of some of our key video conversion results for topic pages:


The columns show data for "supertopic" pages (such as Algebra), "subtopic" pages (such as Solving Linear Equations), and "content topic" pages (such as Finance, which has no subtopics) as well as the homepage topic browser for comparison. The top three rows show the percentage of visitors who clicked on, started, and completed a single video. The bottom three rows show the retention rates between steps.

There are two things to notice here:
  1. The retention rates after the visitor has clicked on a video link are marginally better than on the homepage. This seems good.
  2. The click-through rate is quite a bit lower in some cases than the topic browser on the homepage. This seems bad.
These two contradictory trends roughly cancel each other out in the final number of visitors completing a video! Obviously, we were hoping for a significant improvement in overall conversions and we aren't seeing one.

< the analysis >

Our general attitude at Khan Academy is to ship often and adjust course as necessary, and this is a tacit admission that no plan survives contact with the user base. The thing that has amazed me more than anything else about this is how similar the conversion numbers are even when comparing between completely different pages and navigation styles! While the numbers aren't an obvious home-run, we can see that those visitors who found a video were more likely to watch it, which is a sign that indeed they are more easily finding the video that they are looking for. And while the slightly lower initial click-through rate could be improved, the number of visitors to the topic pages seems to be rising over time, and with it the total number of clicks on video links from topic pages.

It's also important to step back from the numbers for a bit and look at the big picture: Topic pages give us more flexibility for future features, they are a better experience for navigation than the homepage, and we can get massive SEO benefits from of them. Analytics is just one part of the decision-making process, and one we are taking with a grain of salt while we continue to try to better understand it. I'm really happy with how this project turned out and look forward to sharing our next amazing contributions to online education.

< / tom >

Wednesday, February 22, 2012

Today's topic? Topics!

< reorganization >

Last week we at Khan Academy revealed the first pieces of a long-term project to reorganize the content on the homepage and throughout the site. You may have noticed the removal of a number of playlists, notably "Developmental Math" and "Pre-Algebra", and the appearance of newly organized topics called "Algebra" and "Arithmetic and Pre-Algebra". We have grouped the videos under each topic into subtopics as well, to better expose how the thematic building blocks come together and the logical ordering between them. We think showing the structure and consolidating related subtopics under one supertopic is a big improvement over the somewhat haphazard collection of playlists that had evolved over time. Here is what one of the new topics looks like:



The shift in terminology from "playlist" to "topic" is significant: Playlists are completely linear and imply a passive mode of consumption. The concept of a playlist does not include exercises (although the videos in the playlist do have related exercises) and so the natural flow of watching a playlist precludes stopping to do an exercise to practice the knowledge that you've just absorbed.


< topics >

So what is a topic? A topic can represent a single concept or related group of concepts, and unlike playlists they are arranged in a tree. So "Math" is a top-level concept, with "Algebra" beneath it and "Solving linear equations" beneath that. Within these lowest-level topics is our content: Not just videos, but eventually exercises and any other tools that we come up with to teach that topic. The ordering is meaningful so a student can look at the content in a topic and see what relevant videos and exercises teach this concept and the logical order to complete them.

This has both immediate and long-term effects on how students find and use our content. In the short term, we hope that specific and relevant content will be easier to find on the homepage, and there should be less confusion about what concepts that student is actually learning in each topic. In the longer term, we want to encourage a more active and nonlinear mode of participation: students will have the option to interleave videos and exercises for a more active learning session, and students will be able to tackle specific topics by setting goals and getting feedback on a more granular level how they're progressing. This will also help us clean up the Knowledge Map so it's easier to read and navigate.

< api >

One important note for users of the public API: If you have been using http://www.khanacademy.org/api/v1/playlists to get a list of playlists for displaying our videos in your application, you will want to migrate to the new topic tree API: http://www.khanacademy.org/api/v1/topictree. The playlists are becoming less coherent as we reorganize content and will eventually be deprecated in favor of the new organization, and all the new features in the API will be referencing topics. We look forward to seeing what navigation methods you find effective for the topic tree.

< ta-da! >

We are really proud of this architectural and pedagogical improvement. We hope to have all the topics under Math organized in the next few months, and to start rolling out exciting new features that leverage this infrastructure soon. Watch this space, and please leave your feedback in the comments!

< / tom >

Friday, February 10, 2012

Caught in a web: Transitioning from native to web development


About four months ago, I joined the development team at Khan Academy. Working at Khan Academy has been amazing: every day I work with talented, motivated and intelligent people to bring innovative solutions to the nascent field of online education. However, that's not what I'm writing about today.

Before I started this job I worked in computer game development, and my programming language was C. Not C++, just plain old C. Now, I have programmed in many languages – assembly, C++, PERL, Java, you name it. But making the transition after five years writing native code for the PC to full-time web development feels like a radical shift, even though in some ways it shouldn't be. I am setting out to document these differences to serve as a guide to others making the same transition. I suspect a lot of programmers will find themselves in the same situation over the next few years as the complexity (and profitability) of web applications continues to increase.

Speed
As a programming language, C is as simple and straightforward as it gets. Compared to C++ or Java there is less structure and no built-in extensive libraries of data structures and utilities. Optimization can be done at a dizzyingly low level, with control over data structure size, method inlining and even the assembly itself if you want it. You end up writing your own dynamic arrays, allocators, and networking stacks. A seasoned programmer can save megabytes of memory by shaving a few bytes from a key structure, or speed up traversal of a tree by several orders of magnitude by making sure adjacent nodes are in the same cache line. Some programmers are so proud of their skills in these arts that they flat-out refuse to work in an interpreted, memory-managed language. Luckily, there will always be some demand for bare-metal optimization in embedded devices, real-time operating systems, etc. However, I've found that in many cases the most straightforward way to solve a performance problem in any language is simply to do less work, and that involves asking tough questions about what data you absolutely need when, whether calculation can be done in the background or deferred or on a remote machine, and how aggressively to cache results. These skills transfer very well to web development, and I haven't yet seen a case where performance suffered and there was simply no remedy. It's just a different trade-off: rather than code being optimized until it's unmaintainable, data is heavily cached, increasing the penalty for code changes if caches must be rebuilt or migrated.


Iteration
One great upside of web development is the iteration cycle. A full build of a mature game can take anywhere from minutes to several hours, and most PC games take a minute or two to boot up (longer if they are built in debug mode). This means that the time between making a code change and seeing that change in a running game can be 15 minutes or more. (Anybody who points to MS Visual Studio's Edit-and-Continue feature is invited to try it on a million-optimized-file code base!) Even the most trivial change can take an hour to implement. In web development, the time between write and test is more or less the time it takes to hit Refresh in the browser. This has freed me from hours of compilation and startup time. I can't stress enough how much of a difference this has made for my productivity.


Debugging
Now, when it comes to debugging, things are more of a mixed bag. The Visual Studio C debugger is very capable and has some really powerful features. I can't count all the times I set a data breakpoint and found someone misusing a variable. On the other hand, I can't count all the times a data breakpoint has helped me find a buffer overrun or someone writing to freed memory, things I never have to worry about now. In the case of JavaScript, each browser has its own debugger, all of which seem to have “borrowed” each other's features and all of which seem roughly equivalent. The availability of eval() in JavaScript is both a blessing and a curse: Blessing because I can do pretty much anything I want to the running code in the console; curse because browsers don't handle debugging dynamically generated code very well. Then there is Internet Explorer and its own very peculiar bugs. And if you're debugging server-side code in PHP or Python, I have not progressed beyond spamming to the error log. (If you know of a solution for Python in Google App Engine, please let me know!)


I'll go into more detail in future posts, but to sum up: Even though these languages are difficult to get used to if you're used to native C/C++, they do make some things easy and the development tools are constantly improving. While many PC games have updates spaced months or years apart, we are able to deploy code several times a day, sometimes several times an hour. It's a different software development mentality and one that's exciting and invigorating, because it's not about shipping perfect, elegant, bug-free code - what's important is the inspirational product we are delivering for free to students of all ages all over the world.