I've been working in a strict eXtreme Programming team on a client site, which is very nice. Here's some condensed observations and lessons from the last 2 weeks, with a focus on the JavaScript aspects of our work.

---

I made an observation while pairing with someone over the last few weeks. We've been working on a web application which has quite a lot of JavaScript in it. Before starting work for Pivotal, I would've heard 'JavaScript' and run away screaming, but there are a couple of tools and practices that make it quite tolerable:

Prototype , which gives you some OO constructs in JavaScript, making it easier to encapsulate and write re-usable self-documenting code.

JsUnit , a unit testing framework for JavaScript, which is sponsored by Pivotal.

FireBug , an add-on for FireFox. It makes debugging of JavaScript, XmlHttpRequests, CSS, and XML significantly easier. It's detecting of warnings and errors in JavaScript is an absolute necessity.

Baby steps , the practice of doing the minimal possible to make your test pass, to refactor in very small steps, etc. JavaScript is so loose that you can remove a parameter that used to be a hash, be accidentally passing a number into it, and later when to pass it to JSON.toString(), it will be displayed as an empty hash. JavaScript doesn't care if you pass in only some of the listed parameters, nor does it care what type they are, only that the methods you're calling on them or the usage is appropriate. If you get enough things like that piled up, and you have never seen this kind of weirdness before, you will lose your mind and need to revert to get back to sanity. Working in JavaScript *demands* baby steps for optimal work.

Test Driven Development , a practice of writing a failing unit test first before implementing functionality.

IntelliJ , a Java-based IDE, which is quite magical in its refactoring capabilities. It isn't quite so magical with JavaScript, even with Prototype, but still worth using if you have the cash. I assume their add-on for Visual Studio called ReSharper has similar helpful properties.

Armed with these tools and getting the hang of them is very empowering, and provides a lot of courage to expend. My pair, coming from a team with heaps of legacy C++ code with no unit tests, is a little more timid than I am about refactoring. ( Luis was similar when we first started coding together.) This is very common until you get used to having the safety nets mentioned above, but it's also easy to become gunshy after working on a codebase where you're afraid to change *anything* -- even for a feature, nevermind a refactoring.

When I see a code smell , I take baby steps to get the code green so we can then safely attack the jugular and make it clean. Some people think code smells and refactoring are these amorphous, highly subjective things that don't have a real beginning or end. While many undisciplined coders have hijacked the word refactoring for their own self-serving exercises in code eugenics, most other people who use these practices don't work this way. Code smells are documented in the Refactoring books, as are the refactorings themselves. Generally, there are several small refactorings in one complete transformation. Extract a method, introduce a parameter, extract a base class, pull up a field to the base class, make a test subclass, override a method, extract an interface, etc. At each one of these small steps, my tests are green and I can commit -- this also means I can stop at the completion of any one of these steps if I need to. There is a definite beginning and a definite end, and I don't get stuck in a k-hole .

Trying to do all the refactorings at once and getting utterly lost leads to two things: reverting to get back to sanity and trying again (or giving up), banging one's head against the brick wall trying to get everything compiling again, or banging one's head some more to reconcile the copious new test failures because one of the changes somewhere didn't preserve behaviour and you made so many changes you don't know/remember which one could've caused the problem. (Many times, people will actually delete the tests instead of making them pass. If you actually do this, don't admit it in front of me -- I'm likely not to speak to you again. Just kidding ;>).

So, my pair was a little scared. We were looking at a single class that had grown organically and have several responsibilities. It was pulling things directly out of the UI (via the DOM), transforming them into JSON, and sending an XmlHttpRequest (XHR). It set a callback on the XHR so that when the response came back, a method in the class would update the UI directly (via the DOM). Obviously, this class is doing too much, but where to start?

The first thing that was really bugging me was how this JavaScript file was tied to the HTML, and was therefore very brittle. DOM element references were duplicated in several places, violating the DRY (Don't Repeat Yourself) principle. So, that is where we started -- we wanted to pull out the updating of the UI after the response came back. Thankfully, the methods for doing this were already decently decomposed -- we just had to start moving them. That's right -- we just started moving them. We started with one small one, created a new class for it (deferring the proper christening of it until later), moved the method into it, and then let the IDE syntax checking, test failures, and JavaScript errors tell us what we needed to do next (in that order; running the tests before the IDE says the syntax is correct doesn't make sense). There were missing variables that we now needed as parameters on our new method. We added those parameters, and now the IDE was telling us that the original class was referencing a method that no longer existed. We went up to to the original class, created an instance of our newly extracted class in the constructor, stored the instance in a field, and then changed the references to the moved method to be off of the new instance field. IntelliJ said we were good to go, so we then ran the unit tests for the original object. They all passed. Hooray!

Now what, do we commit? If we commit this, are we expressing our intention and direction well enough? If not, what's the simplest thing we can do to express our intention and then commit? In this case, we felt our intention would be expressed decently if we named our new class appropriately. We were extracting UI-specific elements and encapsulating those details in this new class, which seemed to imply it was a View a la the Model-View-Controller (MVC) pattern. We renamed the class and checked it in. A habit I did personally that we also do is to say what the next step is in the commit message. In this case, the next step was to move another UI-specific method into our new View class.

Which method did we choose next? The one that looked like it would be the simplest. We were wrong in the next choice, it ended up being too complicated, so we reverted and tried again with successful results. Reverting sounds like a waste of time, but because we committed in a small increment, it didn't take us that far back. The upside of the mistake is we learned about some dependencies between the methods we were moving that influenced our next refactoring.

Once we had a couple of methods in this new View class, we noticed the duplication of hard-coded DOM element references. To fix this, we made a field that stored that element, and then added a parameter to the constructor for the new View class. We then added the DOM-element parameter to where the View was being constructed -- in the original class. Wait, you say, you're putting the DOM-specific stuff back into the class you were trying to pull it out from anyways! You're correct, and it's a bad smell. So, we need to initialize the View class somewhere else and pass it into the constructor of the other class. We found where the other class was being constructed, in the global onLoad() function, and created our View there. We ran our tests, which passed, then we committed again.

If the onLoad() is going to be tied to the specific HTML, what's the point? That's a great question. Right now, all these classes are in the same .js file as the onLoad(). That doesn't feel right, so next we extracted the individual classes into their own .js files, leaving the original .js with the responsibility of keeping the JavaScript specific to the page. That is, foo.js contains the JavaScript specific to foo.html. Our new view class is now in View.js and is included in foo.js (or foo.html).

We were now approaching something that is a pretty good design. It all started from identifying that the original class was violating the Single Responsibility Principle (SRP), and starting to alleviate some of its burden by extracting a method relating to one of its responsibilities into a new class. It just took the courage to make the first step, and then everything that came after was a matter of following our noses. With frequent commits, TDD, and decent tools, it made the courage easier to come by. Even without those tools, and in a different language/platform/environment, it just takes a little courage to get started and to keep moving at a steady pace if you have the humility to keep your steps small.

Discussion: showing all 0 messages

(No messages)

>

Post a new message:

---