Getting started in legacy code

This tweet got me thinking about how I make commits:

When I work in legacy code I try to keep my changes surgical if I need to reapply/review etc. Being used to format on save doesn't help.
— Kirill Osenkov (@KirillOsenkov) September 28, 2014

I started to reply in tweets, but decided I wanted a lot more than 140 characters.

Formatting

When I come on to a new project, one of the first things I like to do is bulk format all code. I don't care what the formatting is (your conventions are fine with me), as long as it's consistent an automated. Ongoing cost should be near zero.

What is refactoring?

A refresher on definitions of refactoring:

1. Cleaning up the mess while working on something else.
2. Cleaning up the mess while not working on something else.
3. Highly disciplined manual application of a recipe for known-safe behavior-idempotent code transformation.
4. Automated code transformation with a known-safe tool

Arlo Belshee argues that only #3 and #4 deserve to be called "refactoring". He calls #1 and #2 "rewrites", albeit highly localized. All the problems with rewrites still apply, just on a much smaller scale.

I'm inclined to agree with him, but most of the people I work with are happy to call #1 and #2 refactoring as well. I'm not interested in arguing about semantics, but I do find each of these definitions distinctly valuable.

I always advocate for removing barriers to Refactoring #4. They're unlikely to introduce bugs. They almost always make the code better (while most changes make the code worse). They're usually really easy to reverse if we change our minds later.

Refactoring #4 in traditional environments

My current project is sadly trapped in a non-distributed version control system, without lightweight branching. Code reviews are generally required before each change, but we make an exception for known-safe automated refactorings. For example, I might make a commit with this description:

R#: Extract Method `C.Foo`

The R# prefix indicates that this is a known-safe refactoring executed with ReSharper. The `back-tick` syntax is taken from Markdown. I check this in with the minimum of ceremony.

With practice, I have learned to make dramatic shifts in the clarity & structure of code using only the automated refactorings in ReSharper.

It's still my responsibility to make sure I don't break anything. Some ReSharper code transformations are not true refactorings. It's on me to know the difference, and to use the tool safely.

Usually I'll do this with a specific goal in mind: a feature or bug fix. Once the code is really nicely factored in that area, the behavior change is simple and obvious. I can send that out for code review by itself; doing that CR is really easy. (Sometimes people think I only ever work on easy problems!)

I will Refactor this way when I first come in to an area that I plan to work on. As I understand the code, I'll execute these refactorings to express my new understanding, checking in as I go. I'm much more conservative here, because I don't understand overall the intended design, and don't want to naively introduce an inappropriate structure. I pick the most valuable refactorings that I see. In this mode, if I get interrupted part-way through, at least I have made things better. (Thanks to Woody Zuill and Llewellyn Falco for that insight.)

Because I make these changes quickly, I rarely have to merge with someone else's changes. If they have a hard time merging with my changes, we can easily throw mine away and recreate them.

Modern environments

When I get to use Git or other modern source control, that allows an even better workflow.

First, I make a local branch.

Then I'll refactor as above, where each commit goes in separately. I still use prefixes like `R#:` to call those out. I can go a lot faster, because I can postpone any validation until the end of the process.

If I'm working on a feature or fix, goes in the same branch.

If I'm in a code-review-required situation, this pull request is easy to review if you look at each commit separately. 

If every change in the branch is cohesive, I'll merge like this:

> git rebase master
> git checkout master
> git merge --no-ff --no-commit FOO
> git commit    # provide a summary description

This gives a nice-looking commit history:

> git log --oneline --decorate --graph
*   ae593a5 (HEAD, master) Fix blah
|\
| * 67fa3f5 (FOO) Fix bug #2
| * fa27d00 R#: Rename `X` -> `Y`
| * 3a77461 R#: Extract Method `C.Bar`
|/

Just because I have a private branch doesn't mean I want to spend a lot of time there, though. No long-lived parallel development.

Refactoring #3

I'm still looking for a good way to call these out. What I want to tell you is "I didn't intend to change any behavior here. If you're searching commit history for a deliberate behavior change, look elsewhere. If you see a behavior change here, it was not deliberate." In the past I have used "REFACTOR: " as a prefix for this kind of commit.

other tags

Formatting changes are really dull. No one will ever want to read them. They affect many lines of code, so separating them out is really valuable. I commit them with a "FORMATTING: " prefix.

Sometimes I only change a unit test - not product code - and so there's no way I could be introducing a product code bug. Then I might prefix with "TEST: " (although if I extract a method from a unit test, maybe I tag that as a refactoring - still undecided).

Examples of radical simplicity

With so much historical momentum around complex, poorly-factored code, it can be difficult to know what simple code looks like.

It can help to see some examples outside of your normal experience, to get you thinking in new ways. With that in mind, I really enjoyed this list:

1. every JUnit test must run in under 1 second
2. maximum of 1 if statement per class
3. 0-2 fields per class
4. 0-1 arguments per method
5. 1-3 statements per method
6. fewer than 6 imported classes per class

I'm not saying these should be hard rules, or even that they are ideals. Instead, try refactoring your code to fit within these constraints, and observe the result. Does anything useful appear? (Like a Whole Value?)

Automating ReSharper's Inspect Code

If you write C#, I hope you have ReSharper.

If you have ReSharper, I hope you have tried its "Code Issues" feature. It has lots of good suggestions, and I need all the help I can get.

Staying clean requires running the analyzer over your whole solution. ReSharper doesn't update its results automatically: you have to explicitly click to make it happen. I hate explicitly clicking. It's also slow.

I decided to see if I could run it automatically in a Continuous Integration build server. I have figured out how to do so using TeamBuild on Visual Studio Online (VSO). Here's how:

1. Create a new Repo in VSO. Clone it locally.
2. Add your solution
3. Connect to your VSO repo in Visual Studio.
4. Create the PreBuild.ps1 script you see below, to install ReSharper's command line tools
5. Create the TeamBuild.proj script you see below, to run InspectCode and build your solution
6. Team Explorer -> Builds -> New Build Definition. Select the Process tab
1. Set Build Process Template to GitTemplate.12.xaml
2. Projects = TeamBuild.proj
3. Pre-build script path = Tools\PreBuild.ps1
7. Run the script once on your machine to establish a baseline. Add it to Git.
8. Push

Now, when  the set of Code Issues changes, you'll get notified by the build.

Loading TeamBuild.proj from Gist 1d0e58acb2aeda163df3

Loading PreBuild.ps1 from Gist d5a71230d524965057be

I'd like to find a way to do this in Travis-CI, which would require running these tools on Mono. Let me know if you get it to work.

The last responsible moment may be right away!

In Agile, they talk about deferring work and decisions until the "last responsible moment". Later I will know more (so I can make a better decision), and maybe I won't need the work at all (avoiding wasted effort). There may be other reasons.

Recently I've noticed a handful of decisions that should be made really early. Some examples:

• 1-step build
• unit test infrastructure
• create a Git repo
• 0 warnings
• clean static analysis / lint
• slow build tripwire

Some of these are important because they're easiest if you do them before you have any code. 

For example, I've worked on many projects where build + test time was quite long. The slow edit/build/test cycle makes me less productive, so I'd like to optimize it. But the optimization work is expensive for the same reasons: because verifying the changes is slow. 

.@jaybazuzi In all honesty, my build spits out a warning when it gets above 5 seconds. I occasionally optimize as a result.
— James Shore (@jamesshore) April 29, 2014

The longest build I've worked with was 12 hours end-to-end. We all knew it *could* have been faster, but that optimization work seemed so daunting. I could optimize all day long, and get to 11h55m. 

I know there was a ton of duplicated code in different parts of the system. But refactoring to eliminate that duplication was also hard.

If we had maintained a fast build from the beginning, it would have been easier to keep it that way.

There is a balance to be struck. Maybe you're not sure if the idea you have is actually viable. You may want to spike it, or show a prototype to a user, before investing significantly in the idea. (See Lean Startup).

There are other tasks that should come soon but not necessarily immediately, like pushing that Git repo to another machine (so you don't lose it), or setting up an automatic CI build.

Spike and Extract

Sometimes I have an idea about how I want my code to turn out. In that case, I can use TDD in the conventional way: write a test, fill in the implementation to make it pass, refactor.

But a common situation is that I have only a vague understanding of where we're headed. I don't know how my dependencies work. I don't know how it should all fit together. So:

1. Write a spike in the form of a "unit" test.

It's not really a unit test, because it's not testing a unit. But I'll still use a unit testing framework to write it. And I'll use FluentAssertions and ApprovalTests if they're useful

Like any good spike, it should hit the tricky parts, like an API that is poorly documented.

It may take a while to write. I'll try lots of different things until I get it to work.

2. Extract 'till you drop

Use Extract Method, Extract Class, Move Method repeatedly.

Look at any primitives as potential new classes (Whole Value).

Keep refactoring until things read really cleanly. Avoid generality that you don't already need, though.

3. Keep testing.

Write tiny tests for each new method.

If a method is hard to test, refactor.

If a method is hit by more than 4 tests, refactor.

4. RGR

You now have code that works, passes tests, and has an appropriate structure.

Add new functionality with the standard Red-Green-Refactor cycle.

This is not new.

I'm basically describing "TDD as if you meant it" by Keith Braithwaite, but trying to be gentler.

Five ways to know when you're done refactoring & testing

1. Test until bored. Ask "is there any chance this is wrong?" If so, write a test for it. If not, you're done.
   
2. If the test and the system-under-test would look the same, skip the test, because of DRY.
   
3. If the code reads like a spec, test by inspection: read it and confirm that it says the right stuff.
   
4. If a function has a side-effect-free query, followed by an action based on the results of the query, split them and test separately.
   
5. I'm not very good at figuring out how to design code well, but I can tell when code is hard to test. making code easy to test usually works out well for me. If I have to mock A to test B, that's harder than factoring out B and testing it directly. So, I refactor.

These are all different ways of saying pretty much the same thing.

Testing progress reporting

I was trying to get some code under test.

Originally it had been written as one long method. It was a slow process, so another programmer had added progress reporting. He wisely created a new, decoupled progress reporting service, with an interface like:

Loading IProgress.cs from Gist 9867358

My first step was Compose Method: Applying Extract Method repeatedly until the code read roughly like English.

Loading Example1.cs from Gist 9867358

With the exception of the progress reporting, there's no point in writing a test for a Composed Method. The test would just repeat the code, violating DRY. Test by inspection.

But what about the progress reporting? How do I test it? Some options:

• Don't test it.
• Test manually.
• Use an integration/end-to-end/system test.
• Mock out the Do*() methods in a unit test.
• Change the design.

Each option has pros and cons; each has a context in which it is the right choice.

TDD teaches us that difficult-to-test is a code smell; it's design feedback. All other things being equal, I'd rather choose the last option and improve the design.

There are several ways to think about the problem that could lead us to a new design:

• The number "4" violates DRY with the (often implicit) parameters to ReportWork. How can I test that it stays correct when editing this code in the future?
• Most difficult-to-test problems are solved by decoupling. What could I decouple here?
• This method is responsible for both doing the work, and reporting on the work. Can I write those separately?
• Think about the difficult-to-test aspect of the code; what if I made a new class that did only that, free of any context?

Suppose there was a class that was responsible for calculating & reporting the work to be done. You could just pass it a list of steps, with costs, and let it calculate the sum. The result might look like:

Loading Example2.cs from Gist 9867358

Since lambdas are capable of capturing locals, it's easy to use the output of one step as the input of another:

Loading Example3.cs from Gist 9867358

Consequences

The code is far easier to test:

• DoWork, etc. are more testable now as separate methods, since they were extracted.
• The ActiosnWithCostsList class is trivial to unit test. (You might not bother... up to you.)
• The ExecuteWithProgress extension is non-trivial, but still easy to unit test.
• The original code is now very simple, and should be tested by inspection.

I would like a lightweight end-to-end test to hit this code, just to make sure I haven't missed anything. Maybe that's just because I'm still new to TDD and haven't developed the confidence in my TDD skills yet.

If you're not used to coding this way, it may feel surprising. Once you understand it, it may seem clever. Those are both feelings I want to avoid in my code. (Well, I do like looking clever, but it's still a code smell.) I believe that any competent C# programmer can spend a few minutes reading my tests and understand what's going on here.

The 4 rules place "passes tests" above "expresses intent", so I think it's worth keeping until we find a better solution.

Next steps

You wouldn't be reporting progress if the operation was fast. Since it's slow, you may want to free up the current thread while waiting. That means async/await. Extend this solution to make that easy.

If you call a method that could report fine-grained progress, it would be nice to pass it a child progress. Something like:

Loading IProgress2.cs from Gist 9867358

sorting out complex refactoring with the Mikado method

My understanding of the Mikado method:

Problem: You see a mess in the code so you decide to fix it. Part way through you realize you gotta fix some other thing first. So you start working on that, which leads to more.

Now your code is all torn apart and you're late for dinner and nothing works.

Solution: When you realize you can't finish problem A until you fix problem B, write down A and abandon your changes so far. When B leads to problem C, write down B and abandon those changes, too.

You're discovering a dependency tree. Eventually you'll get to the leaf nodes. When you fix a leaf node you don't have to fix something else at the same time. So fix it and commit. Now you've accomplished something.

Each commit prunes the tree. Eventually you get back to your original problem, which you can now safely fix without further difficulty

I think this is applicable to other problems besides software, like organizing your kitchen or car maintenance.

Conways Game of Life in Gherkin

I am trying to learn Gherkin, and needed a problem to practice on, so I chose Game of Life. I think my current result is interesting enough to post.

Loading Gist 9454787

I'm struggling at the moment trying to figure out the right level of detail and correct organization in the tests. I'm curious what I'll think of my attempt a week from now / a year from now.

Stages of TDD

I've often heard people say this sort of thing about TDD:

I really believe in the importance of TDD. It helps me catch stupid bugs right away, instead of waiting for testers or customers to report them. I especially like that it catches any regressions.

TDD lets me refactor safely. Also, writing tests first help me think about my design from the caller's perspective.

I still need a comprehensive suite of end-to-end tests to make sure the whole system works. I am concerned that, since I'm writing both the code and the tests, my blind spots will appear in both, allowing bugs to slip through. 

When requirements change, or we refactor a subsystem, a thousand tests break. Then we have to spend a lot of time fixing them.

TDD is expensive, but it's worth it.  

While others say something like:

TDD is not a testing activity; it's a design activity. "Test" is a misnomer. With TDD I write better code, faster. 

I don't need a bug database. My whole team only has a couple bugs per month. We do root cause analysis on every bug.

From the first perspective, the second sounds weird. It's hard to believe it's even possible. Maybe it only works in that specific context.

From the second perspective, the first sounds backwards and unenlightened. Why don't they get it?

I now believe that learning about TDD takes time and moves through stages. From the early stages, it's hard to fully understand the later stages. From the later stages, the you wonder why you ever wasted your time in the early stages.

The stages that I see are:

1. TDD is about testing. I write tests to prove that my code works. Thanks to the safety afforded by those tests, I refactor more.
   
2. I commit to 100% TDD. Every feature and every bug fix will be represented in tests. I immediately discover how hard this is, which leads me to mocks, dependency injection, marking methods as "public" for testing, etc.
   
3. Tests give me feedback. I see "hard to test" as a code smell. I refactor to make things easier to test. Tests get easier and code gets cleaner.
   
4. My tests are always easy to write, easy to read, and super fast. Code is clean. Every idea has a single canonical location. There is no duplication. Classes have great names. Cohesion is high and coupling is low.
   
   There are no bug farms, no part of the code I'm afraid to touch. Working in this code is inherently low risk. Since I rarely create bugs, testing for correctness is rarely fruitful. So yes, tests got me here, but not by catching bugs.

I found #4 very hard to grasp a year ago. I'm not sure if seeing this roadmap would have helped. Perhaps the right way to begin is just to focus on #1: write tests to catch bugs. Make developers responsible for proving correctness of their work. Don't count a feature as "done" until the bugs are gone. Build from there.

There may be an additional stage in the middle. If you know it, tell me and I'll fill in.

I'm really curious if there's a stage 5. Something I'm blind to. Got any?

Various definitions of "Refactoring"

In conversations with other programmers, I have heard people use "refactoring" to mean four different things.

0. (Not refactoring). Making the minimum necessary change.

This is often expedient and may feel safest, where safety = not breaking existing functionality, and not getting yelled at.

1. Doing more than the minimum necessary change.

You could hack in your bug fix or new feature. Maybe that's hard because the code is already convoluted. Or maybe your hack would make the code convoluted. So you clean things up a bit at the same time.

2. Cleaning up code without changing existing behavior.

At least, you hope you're not changing behavior.

3. A highly disciplined process of known-safe code transformations.

Within a method body I feel confident that I can rename a local variable with Search/Replace if I first check that the new name isn't already in use.

Feather’s book (aka WELC) includes some highly-detailed methods of doing this to legacy code to get it to the point where you can start writing unit tests. By detailed, I mean tedious.

4. Using a mechanized refactoring tools.

For example, select a line of code, RClick, Extract Method.

Only #3 and #4 are safe enough that I do them without fear.

My preference is to do #4, over and over again. (Also, I commit each one separately.)

---

Some will say that #1 and #2 aren't "true refactoring". I find that arguing about definitions to be counterproductive and off-putting. Each of these activities have value in some context, and each are worth discussing. However, it should be noted that Martin Fowler is one of those people.

Variation in test terminology

When I observe conversations about testing with other engineers, I notice that latent disagreements about the meaning of testing terminology breaks the conversations. Latent, because people think they're talking about the same thing, but they are not.

Here are some examples:

"Integration Test" / "Integrated Test" - is there a difference?

• I have tested Thing 1 and Thing 2 in isolation; now test that they work together.
• A test for Thing 1, but Thing 2 is along for the ride.
• I test the entire system, end to end.

"Unit Test"

• Any test written by developers
• A test that talks to the code directly (any size of "unit")
• A test of a small portion of the code
• A test of a single class, in isolation
• A test of a single, tiny behavior

TDD

• A practice where developers write automated tests
• Tests get written before code
• We look to tests for feedback on our code design
• I follow a RED/GREEN/REFACTOR workflow

UI Test

• I test my business rules by manipulating the UI
• I test my program as a whole by manipulating the UI (assume the components are already tested)
• I test my UI

Why unit tests?

In conversations around unit testing, I regularly hear someone assert the real reason we write unit tests. Sometimes the conversation goes like this:

Alice: "We should write unit tests"
Bob: "No. We can already achieve result X through some other approach." or "No. Unit tests fail to accomplish Y which is important."
Alice: "But Z is the real reason we write unit tests."

It would help me to have a list of supposed reasons for unit tests, so I'll catalog them here.

Note that I'm not asserting specific definitions of "unit" and "test" and "unit test", although varying definitions here are a big part of the problem.

Correctness: proof that my code does what I intended it to do.

Detractors: There are many bugs that unit tests won't catch, e.g. integration bugs, security bugs, timing bugs. Also, the mindset that created a bug is also the mindset creates the test, so we can't rely on unit tests to catch all the bugs.

Proponents: Developers often fail at basic correctness, leaving testers the tedious work of finding easy bugs. By using unit tests to ensure basic correctness, even imperfectly, testers can do the interesting and important work we need them to do.

Regression: proof that my changes didn't break something else.

Proponents: Much time and energy go to fixing regressions, which could be eliminated if we had tests.

Detractors: Same arguments as in correctness.

Refactoring: I can safely refactor without introducing regressions.

Proponents: Refactoring is key to the long-term well-being of the code base, and the mental health of the programmers. Having unit tests in place makes that safe.

Detractors: The obvious arguments from above, plus the burden of dealing with failing tests whenever you change something that should be innocuous. Do you fix 100 tests or just throw them away?

Design: unit tests help me reduce coupling & increase cohesion

Detractors: I don't see that happening.

Proponents: When a test is hard to write, that's design feedback to refactor your code. When the test is short, clear, easy to write, easy to read, has a good name, and runs fast, you know your code is in good shape.

Note that it can take a lot of practice to develop the skills required here.

Efficiency: Unit tests affect the time it takes to get the job done

Detractors: Time spent writing unit tests could be time spent creating customer value. Also, writing these unit tests takes a long time.

Proponents: Highly skilled unit testers write less code because they have less redundancy and they only implement as much functionality as is required by the tests. They also spend less time fixing bugs, and can afford to perform root cause analysis on every bug.

Logging an Item

MSBuild items seem like the should be almost the same as properties, but they are different in some odd ways. Time to add support for them.

Most of the time was just spent iterating with MSBuild, trying to understand what it expected, and how it would respond to different inputs. I'm glad I'm encapsulating that here in this class, so the rest of my Task code doesn't have to worry about it.

Loading Gist 7523672

EDIT: After posting this, I noticed a bug. I had hard-coded the name on line 7 of LogProperty. Some TDD gurus insist that you write more than one test before removing hard-coded results. If I had done that, I would have caught that bug before committing my code. I'm tired, so I'm looking for shortcuts; if I had a pair, I bet we wouldn't have let that happen.

Refactoring the test

All the TDD gurus admonish us to keep our test code well-factored.* I know I'll want to write more tests soon, so now is a good time to clean up the test I have. Let's refresh ourselves with starting code:

Loading Gist 7519763

This is a job for Extract Method. Most of the effort goes in to picking the right places to slice the method in to bits.

It's easier to know where to slice when you have a second (or 3rd!) example - it's easier to see the duplication. On the other hand, refactoring 3 is more work than refactoring 1. I took a quick stab at writing a new test, saw the duplication, deleted it, and then refactored. Call this an educated guess.

Here's what I ended up with:

Loading Gist 7522819

I'm curious about this new assert. Some TDD gurus mention custom asserts as one element of advanced TDD, so I am going to pay attention here and see if anything interesting happens.

Next up, a new feature, with a new test. Let's see if my refactoring works out.

*Arlo Belshee goes a step further: don't worry about duplication between tests as much as making the rests read cleanly. They should say exactly what you want to test, no more, no less. He says tests should be "WET" - Write Explicit Tests, while product code should be "DRY" - Don't Repeat Yourself.

Refactoring the logging

Now that the tests are working OK, I want to turn my attention back to the other goal: cleaner code for task parameter logging. Here's the current implementation:

Loading Gist 7517928

I want to refactor this to generalize. I'm not going to add additional tests just yet, even though maybe I should. At least I have enough test coverage that I don't have to spend time manually testing as I go.

I used very small steps, but the outline is:

1. Extract Method LogInputs, push it up to base class LoggingTask
   
2. Replace the hard-coded "Foo" and this.Foo with reflection via this.GetType().GetProperty("Foo").Name and .GetValue(this).
   
3. Extract method LogInput, taking a PropertyInfo
   
4. Replace GetProperty() with a loop over GetProperties(), filtering by the [Required] attribute.
   
5. Extract Method GetPropertyInfos and HasAttribute.

Here's the result:

Loading Gist 7521986

As an indication that I'm on the right track, see how small the test task is:

Loading Gist 7521995

You will also need this extension:

Loading Gist 7522018

I don't really like that he [Required] attribute is used to find parameters; that's not quite the right rule. But I'll come back to that later.

Creating the MSBuild project in-memory

Now that we're no longer running the MSBuild command line, the next isolating refactoring is to create the project file in memory, too. To do that, I used ProjectRootElement:

Loading Gist 7519460

Note this depends on the following extension method, which I introduced to reduce duplication:

Loading Gist 7519477

I have an urge to write an extension method to replace AddTask which will ensure that the project has a UsingTask already. In fact, I'll go ahead and do that, so you can see the transformation.

Loading Gist 7519648

I also made AddUsingTask generic:

Loading Gist 7519677

Which allows the following simpler test:

Loading Gist 7519763

Running MSBuild in-proc in a unit test

In the previous step, I executed my code under test (a custom MSBuild task) by running MSBuild as a separate process, with a hard-coded path to the EXE, passing a project file on the command line, and capturing standard output. An ideal unit test runs the system-under-test (SUT) in isolation, and this is far from isolated. That's fine in spike mode, but over time I want to refactor my tests to improve isolation.

To take a step in that direction, I'm am switching from Process.Start to using the MSBuild APIs directly. It took a while to figure out a way that works, but now I've got one:

Loading Gist 7519022

This requires a custom logger to gather the output, which I wrote like this:

Loading Gist 7519004

This new logger  is pretty specialized - high importance messages only. I have an urge to generalize it to let the importance be a parameter, but for now, I'm applying YAGNI and leaving it as simple as I can.

Running MSBuild in a unit test

My aim now is to write a simple, fast, reliable test that will execute my custom MSBuild task. Currently, I can run it with the command line:

PS> msbuild .\My.proj /v:d

One challenge with testing it this way is that the output has a lot of non-repeatable test, including timestamps. Just to make things easier, I'll elevate the logging importance to "high".

Log.LogMessage(MessageImportance.High, "  Foo = {0}", this.Foo);

Then I'll run with minimal verbosity, and nologo to clean up the output:

PS> msbuild .\My.proj /v:m /nologo
  Inputs:
    Foo = hi

Now I need to actually write a test. The simplest test I can think of:

Loading Gist 7518446

Note that I wrote the expected output as empty, to see the exact actual output (which is already known good, since I'm writing the test after the fact). I'll grab that actual output, make it the expected value, and the test will pass. That way of working is the basis of Llewellyn Falco's ApprovalTests, which I may switch to in a future step. But for now, make the test pass & check in:

            const string expected = @"  Inputs:
    Foo = hi
";

spiking a custom MSBuild Task

My team has a bunch of MSBuild tasks. Most of them write their input parameters to the log, like this:

Loading Gist 7517928

As of recent versions of MSBuild, this logging already happens automatically, but only in diagnostic logs. Those logs are so big as to be unusable for us, so we want this information to appear in the detailed (or perhaps normal) logs.

Today, I want to explore two ideas:

1. Using TDD on MSBuild tasks, and
2. Removing duplication of this logging code.

I'll start with a spike, to confirm that I do indeed know how to write & run a custom MSBuild task. The task implementation is presented above. Next, I write a small MSBuild project:

Loading Gist 7518019

And then run it on the command line (the 2nd run is with diagnostic verbosity, so you can see what the built-in logging looks like)

Loading Gist 7518058

Great, I know how to do something. :-)

Next up, turn this in to a test.

TDDing to ILock

My guiding star: when faced with a difficult TDD problem, refactor to make it easy. There are many techniques for doing so (I only know a handful).

What if we use dependency injection?

1. Wrap the `lock` keyword in a custom class.

2. Pass in that custom object

3. In tests, pass a mock of that custom object instead.

Here's the resulting implementation of Counter:

    class Counter

    {

        readonly ILock @lock;



        public Counter(ILock @lock)

        {

            this.@lock = @lock;

        }



        internal int MoveNext()

        {

            using (this.@lock.Acquire())

            {

                return CurrentValue++;

            }

        }



        public int CurrentValue { get; private set; }

    }

One way of looking at what I've done here is addressed the Primitive Obsession around the _syncRoot object in the conventional pattern. You can't get much more primitive than System.Object!

You can see the resulting code here: https://github.com/JayBazuzi/TddLocks/tree/IDisposableLockToken/LocksExperiment1/LocksExperiment1

One thing to look at is my commit history: https://github.com/JayBazuzi/TddLocks/commits/IDisposableLockToken/LocksExperiment1/LocksExperiment1

Note that I explicitly called out which steps were RED, GREEN, and REFACTOR. 

5b36285 REFACTOR; Rename `MyLock` -> `Lock`

c149f15 REFACTOR: Rename `Lock()` -> `Acquire()`

66518d7 GREEN: Simple `Lock()` and `IsLocked`

9b27a8c RED: LockedLockShouldBeLocked

Also, as the code currently stands (https://github.com/JayBazuzi/TddLocks/tree/IDisposableLockToken/LocksExperiment1/LocksExperiment1), there are 3 implementations of ILock:

MonitorLock, which attempts to duplicate the behavior of the `lock` keyword in C#, according to the language specification.

MockLock, which makes it easy to confirm that code is locking as expected.

SingleThreadedLock, which actually doesn't lock at all.

I created the different Lock implementations with TDD. All 3 pass the exact same set of unit tests.

Fun stuff.

Conventional threadsafety

The conventional approach is to use the `lock` keyword to wrap operations that are sensitive to threading.

    class Counter

    {

        readonly object _syncRoot = new object();



        int i = 0;



        internal int GetValue()

        {

            lock (_syncRoot)

            {

                return i++;

            }

        }

    }

It's popular, and there's nothing wrong with it per se, but my goal is to use TDD. I can write a test that motivates the non-lock parts of this code, but I don't know how to write a test that demands the locking.

TDD and multithreaded code

I recently watched a video of a talk by Venkat Subramaniam called "Test Driving and Unit Testing Thread Safety". http://www.agilealliance.org/resources/learning-center/test-driving-and-unit-testing-thread-safety

My first understanding of the title of the talk was that Venkat was going to describe a novel way to test code for thread safety. That understanding makes sense if you think of unit testing as a way to measure & ensure the quality of code.

However, I keep having to remind myself that TDD is not primarily a testing activity; quality assurance is not its primary goal. The primary purpose of TDD is design - to help you write well-designed code. That works because you can't test things in isolation if coupling is high.

Any time you're doing TDD and have code you don't know how to test in isolation, you have 3 options:

• Don't test it.
• Test it in context, in an integration test / manual testing / etc.
• Refactor.

That last one is the gift TDD offers, and that's what Venkat was recommending in his presentation.

As I was watching Venkat code, I realized that what he was creating looked very familiar. I went back and found this blog post from 9 years ago: http://blogs.msdn.com/b/jaybaz_ms/archive/2004/05/06/127480.aspx. What's interesting to me here is that we came up with that code without doing TDD. We were only coding for fun, and we didn't understand TDD at the time. We just refactored mercilessly and that's what we ended up with.

I've also been reading Arlo Belshee's blog. He talks about mocks as a code smell, and the pattern of using simulators instead of mocks for external dependencies.

Thinking about Venkat's talk, Arlo's blog, and my old blog post, I decided to attack this problem again as a kind of kata. Specifically, I wanted to practice some ideas I've been studying recently:

• Strict RED/GREEN/REFACTOR
• Work Tiny.
• NCrunch.
• Letting TDD drive the design.
• Treating mocks as an invitation to develop multiple useful implementations

Problem statement

Write a threadsafe counter. It supplies sequential integers to callers from multiple threads.

That is, make this threadsafe:

    class Counter

    {

        int i = 0;

        internal int GetValue()

        {

            return i++;

        }

    }

(https://github.com/JayBazuzi/TddLocks/blob/master/LocksExperiment1/LocksExperiment1/Counter.cs)

Single-line blocks: A radical view

For a long time I've been a staunch advocate of using braces around single-line blocks in C-like languages:

            // OK
            if (foo)
            {
                return 1;
            }


            // Bad
            if (foo)
                return 1;

The obvious reason is to protect against this kind of bug:

            // very Bad
            if (foo)
                Debug.WriteLine("returning 1"); // added later
                return 1;

There's actually a bug in Visual Studio that we shipped because of this kind of mistake. If memory serves, it was in VS 2002. The repro:

1. Start debugging.
2. Open the watch window
3. Make a Remote Desktop connection to the session running the debugger

Result: 

• The font in the watch window changes to the system font (big & ugly).

The problem here was a bit of code in DllMain of a debugger DLL where a single-line block didn't have braces, and then another line was added later. Turns out DllMain gets called when connecting in a RDP session. The Watch Window font gets deleted prematurely. This was in the early days of Terminal Sevices / RDP, so it took us a little while to get cluefull (opposite of clueless!).

It would have helped if we auto-formatted our code, but we didn't like what the C++ auto-formatter did by default, so we didn't run it.

Since then, I have argued that all single-line blocks must have braces, to prevent this kind of bug.

Later I started reading about Extreme Progamming, and their radical views on simplicity. I decided that simple, single-line blocks are a nice goal, because they mean your code is simple. So, I now have an exception to my rule. A single-line block can skip the braces if 1) it is on the same line as the if/for/whatever statement, and 2) the whole line is short. Also, I consider this state of affairs is particularly desirable.

So:

            // Good

            if (foo) return 1;

            else if (bar) return 2;

            else return 0;

A way to manage extension methods in C#

I like C#'s extension methods, but I'm concerned about "polluting the namespace". I'm not the only one. I'm especially concerned about extensions to "object", as they appear everywhere. They can be useful, but wow that's a big impact. How to make valuable, wide-reaching extension methods possible without affecting lots of code that doesn't need the extension?

I've come up with an approach that helps me be picky about which extension methods are available in which contexts. That helps me manage how much pollution I am willing to accept. It is also means that most of the extension methods available in a given context are the ones that are relevant to the code I'm trying to write in that context.

Example:

namespace MyProject.Extensions.TypeExtensions
{
    static class _
    {
        public static bool DerivesFrom(this Type type, Type baseType)
        {
            while (type != null)
            {
                if (type.BaseType == baseType)
                {
                    return true;
                }

                type = type.BaseType;
            }

            return false;
        }
    }
}

Which is normally used like this:

    using Extensions.TypeExtensions;

    var typesDerivedFromFoo = from type in typeof(foo).Assembly.GetTypes()
                              where type.DerivesFrom(typeof(Foo))
                              select type;

Consequences

* That this approach works best if files are small, which means that my classes need to be small, too. If all your code is in a few files, there's no point to my method.

* All my extension methods are in the Extensions namespace.

* It's not friendly in languages that don't have extension methods. (You have to use the full name of the extension type):

Extensions.TypeExtensions._.DerivesFrom(type, baseType)

This isn't beautiful, but let's compare it to what you'd write if you didn't use extension methods at all:

TypeExtensions.DerivesFrom(type, baseType)

There is only an extra "_." and an extra "Extensions.", which isn't that much.

* My approach prevents you from using these methods as if they weren't extension methods, the way we did before:

using Extensions.TypeExtensions;
_.DerivesFrom(type, baseType);

Because "_." is ambiguous as soon as you bring in more than one extension.

* While C# requires that extension methods be defined in a class, the language doesn't care what the name of the class is. My approach reflects that, by giving the class a "nothing" name (and using the containing namespace as the visible name).

* The word "Extensions" appears twice in the namespace name, which is repetitive, redundant, says something that was already said. I'd rather call it "Extensions.Type", but that means that the name of type I'm extending ("Type") isn't available - I have to say "System.Type". Maybe that's a better choice.

Version control that scales down: Mercurial

Version control that scales down: Mercurial

 

When working on a small software project for myself, I still want version control, but I don't want the overhead of, say, TFS.

 

I decided to give Mercurial a try, and it seems to scale down very well.  Here's what I have done:

 

1. Install TortioseHG
2. Open a PowerShell Prompt
3. Go to the directory that contains my source
4. hg init # create a new "repository"
5. hg addremove # add all files in the current directory
6. hg revert # avoid adding files you don't want
7. hg commit

 

A good idea is to create a .hgignore file to tell it what files you don't want to add, if there are some.  For a small C#/NUnit project, mine looks like this right now:

syntax:glob

*.suo

*.user

bin\

obj\

TestResult.xml

NUnit.VisualState.xml

 

Then, to add this file, do hg addremove, then hg commit.

 

Then, you just edit your files as needed.  When you're ready to commit, you do hg addremove, hg commit.

 

I don't have to worry about keeping my filesystem and my project in sync with my source control.  It just does it.

 

In another directory I have some PowerShell scripts.  Since I already had Mercurial installed, it was easy: hg init, hg addremove, hg commit.  Tada, it's under version control. 

 

Things I like about Mercurial:

 

• No server setup
• No need to decide where on the server your files should live
• No need to "check out" a file before you can edit it - adding version control doesn't interrupt your existing workflow.
• Works offline just as well as online

 

Mercurial has a lot more power if you want to scale up, with rich branching & merging.  You can create a branch for an experiment or a 1-off bug fix release, all offline.  That's cool, but right now that's not important for me: I really just want to track my changes.

 

The main thing I wish for, and it's pretty minor, is PowerShell cmdlets.  They're really nice to work with.  I don't expect them to appear any time soon.

 

People often look to version control software to provide backups of their source code.  I only have Mercurial on one machine right now, and standard guidance says I should put the small Mercurial server on another machine (my Windows Home Server seems like a good choice), and "push" my changes on to it regularly.  I'm not doing that, because my changes are backed up in other ways:

 

• The directory that contains all my source code projects is in a Windows Live Mesh folder.  That means it's backed up, even the full history, unless I accidentally delete the whole thing.
• Every night my computer is backed up to my Windows Home Server.  Even if I delete by accident, I won't lose much.

 

I'm not sure how well mesh & Mercurial will get along.  If I sit make edits on computer A, and Mesh syncs to computer B, I could commit from B, and that will get synced back to A.  That isn't what Mercurial is intended to do, but I can't think of a reason it wouldn't work.  The idea of being able to go computer hopping without having to first commit, push or pull, merge, commit each time seems attractive.

 

Backup up your pending changes (TFS)

More than once I have destroyed my data by accident. I've certainly lost more data this way than any other. Version control is great for a lot of reasons; having a backup is just one of them. But if I have changes that aren't checked in, they are at risk. When I was working with TFS for version control, I wrote this PowerShell script to back up all my changes to shelvesets. TFS has the ability to enumerate all workspaces, so it's easy: you just run it once and it will back them all up. The shelveset names begin with "ZZZ" to sort them to the end of the shelveset list. It makes sense to run this as a nightly scheduled task. The script is also interesting as an example of how to manipulate TFS from PowerShell, via the TFS APIs, instead of trying to parse textual output:

[void][System.Reflection.Assembly]::LoadWithPartialName("Microsoft.TeamFoundation.Client")
[void][System.Reflection.Assembly]::LoadWithPartialName("Microsoft.TeamFoundation.VersionControl.Client")

$localWorkspaceInfos = [Microsoft.TeamFoundation.VersionControl.Client.Workstation]::Current.GetAllLocalWorkspaceInfo() | where { $_.Computer -eq $env:COMPUTERNAME }

"Found {0} workspaces to back up" -f $localWorkspaceInfos.Count | Write-Verbose

Download:

Warning: it has been a year+ since I last ran this script, and I don't have access to TFS these days to test it. I think it works, but YMMV.