Macadamian Blog

4 Ways To Review An Estimate With Limited Time And Limited Expertise

At last week's OSEF meeting, a scenario was described: you've planned a technically complex feature in your next release, and your senior developer has researched it and put out an estimate of 6 weeks to complete. Of course you trust your developers, but anyone can make a mistake. Without knowing anything about about the technical aspect of the new feature, and with limited time to review, how can you verify if the estimate is at all accurate?

1. Review The Work Breakdown

The most accurate type of estimate is a bottom-up estimate, so hopefully your developer broke down the feature into a list of tasks and sub-tasks. And the finer the granularity, the better. Are there any tasks that seem too high-level, like "Develop the UI" at 15 days, or "Multi-Thread Mechanism" at 10 days? At Macadamian, any sub-task estimated at 4 days or more is considered suspicious and should be cause for a bit of Q&A. What is really going to happen within that 15 day task? Couldn't it be broken down further into smaller sub-tasks? Without even understanding the technology, a granular breakdown of sub-tasks is already a good sign of a well-thought estimate.


2. Review The Top 3 Risks

Ask the estimator to list the top 3 risks of the feature. Often, we think only of the success path when estimating, overlooking everything that might go wrong. When it comes time to implement, the risks rear their ugly heads and the success path, and the accuracy of your estimate, is ancient history.

Have a look at the 3 risks. Are they clear and detailed? Are they really the top 3 risks, or can you think of other bigger risks? What is the probability of occurrence? What is the impact if they do occur?

What are the proposed mitigation strategies? Are they taken into account in the task breakdown?

3. Review The Non-Technical Tasks

When focusing on a complex technical feature, it is easy to forget about the "obvious" non-technical tasks - management tasks, coordination, unit testing, bug fixing, answering questions, integrating with other features, documentation, contacting 3rd party vendors, etc.

Is setup of a nightly build part of the estimate? What about the integration of tools like NUnit and FXCop? How about meetings that can add up to several hours per week? Time to integrate 3rd party vendor fixes? Time to merge the branched code into the main branch of source control?

They may be small in comparison to implementing a complex thread dispatcher, scheduling algorithm or power management scheme, but when you add up all these little half tasks, they can actually account for a significant chunk of the estimate.

4. Hold A Brainstorming Session

Although I typically recommend a single peer to review code as opposed to a whole group, when it comes to estimates I believe the more the merrier. Different people have different background, different roles within the company, different concerns, etc. Holding just 1 brainstorming meeting when an estimate is first drafted can bring up all kinds of new issues that no one ever thought of. In fact, I would say that including more of the team in the estimate review is the 1 major factor that has improved our estimation accuracy.


Of course, there are all kinds of processes and software out there that can improve your estimation accuracy: function points, Delphi technique, risk estimation calculators, Monte Carlo simulation, etc. As a product outsourcing company, accurate estimates are crucial to our business, and we're always looking for ways to tighten them up further. But additional processes can take big time and cost big money - not something that you can always justify for the work at hand.

The above tips are designed to give you the most benefit in a short amount of time. In fact, you could probably apply all 4 tips in 1 day, so you can get started on the actual development ASAP.

A Patch A Day Keeps The Rework Away

I met with Ovidiu this week, the new developer hired in our Romanian office, to go over the material of first week training. Once you start development, I said, we have a requirement that you must submit "1 patch a day". Silence on the other end of the phone, a not uncommon response from any new team member, local or global. Ovidiu speaks English very well, but in this case it was like I was speaking a different language. "Let me explain what it's all about - you've heard of test-driven development, you've heard of pair programming. Well patch-a-day may just be an even stronger development process."

What is a Patch?

A patch is a portable file which contains new code that a developer wants to add to source control. It might be one new function. It might be a bug fix. It might be a skeleton of a new class. The rule is that it should be small and self-contained.

A developer works on this code on his local machine. When he is ready to submit the code to source control, he generates a .patch file, a text-based file that lists the lines of code that were added, deleted and modified. The patch file can be used to revert or re-apply the changes on the developer's machine. The patch file is portable because the developer can give the file to other developers on the team and they can apply the patch to their local copy of the code.

Index: comp/ajax/signup_user_prepare.php
==============================================
@@ -14,18 +14,48 @@
if (isset($_REQUEST['vl_id'])){

- $sm->assign("vl_id", $_REQUEST['vl_id']);
- $sm->assign("ta", $_REQUEST['ta']);
- //$content = $sm->fetch("signup_ajax.htm");
-
- $content = $sm->fetch("signup_ajax_player.htm");
+ $sm->assign("vl_id", $_REQUEST['vl_id']);
+ $sm->assign("ta", $_REQUEST['ta']);
+
+ $content = $sm->fetch("signup_ajax_player.htm");

Many source control systems (CVS, Subversion, etc.) have an option to generate patch files. Others can be adapted to generate these files or similar.

Why Use Patches?

Why even use patches? Why not commit the code directly to source control?

In Macadamian's process, the source control must always be 100% functional. A customer request for a working version of the code, a tester's request for a last minute bug fix, a request to release an application update could come at any moment. Team members must always have access to the latest copy of working code. Nightly automated unit tests must all pass on the integration server. If the utmost quality in the source control is not preserved, then it becomes a bottleneck.

The second reason to use patches is that they facilitate code review. The process works like this:

1. Developer sends an e-mail to the project mailing list, containing:
   
   • the patch file attached
   • a brief explanation of the changes
   • the name of one team member who will peer review the code
2. The reviewer and and developer then discuss the code publicly on the mailing list, refining it as necessary
3. Other team members, the architect, the manager, etc. are free to comment or do ad-hoc reviews on the code
4. When all are in agreement, the patch is committed to source control

The patch file is easily readable by the readers of the mailing list, either in plaintext format or by using special viewers like WinMerge to do highlighting. Having the patches attached and sent out gives the team no excuses not to open up the patch and do an ad-hoc review.

Why Every Day?

Submitting a small patch every day has several advantages:

• small patches are easy to understand, so you'll increase the changes of getting a quality code review (if you dump a huge amount of code, the reviewer may miss things or worse, skip the review altogether)
• frequent patches mean that problems will be caught early, before the developer goes too far in the wrong direction. This is especially important when your team is made up of team members in different time zones.

Every Day, Are You Sure?

What if my function is not finished within a day? What if my class is not completely implemented? I can't submit a patch unless my code is pristine, right?

Yes you can, you must, and in fact, it's easy using the power of stubs and todo's.

A stub is a class or function which has little or no implementation. In order to produce a patch a day, when you start a new class, your first patch is probably going to be a stub:

class LoginDialog
{
bool validateUsername()
{
// TODO: validate the user name length, valid characters, etc.
return true;
}

bool validatePassword()
{
// TODO: validate the password in the database
return true;
}

void doLogin()
{
// TODO: 1. pop up a login dialog
// TODO: 2. get and validate the user credentials
// TODO: 3. login to system
}

string username;
string password;
}

With a stub, your first patch really just outlines the design of your class. You submit this code for review - it compiles, it will not hurt the source control, and it's small enough for the reviewer to understand easily. Maybe you need a function to get the database instance? Maybe it's a security risk to store a password variable? The reviewer is not concerned about implementation, he is concerned about reviewing the design, and catching problems early.

The next patches will replace the TODO's. Perhaps the next patch implements the login dialog:

class LoginDialog
{
bool validateUsername()
{
// TODO: validate the user name length, valid characters, etc.
return true;
}

bool validatePassword()
{
// TODO: validate the password in the database
return true;
}

void doLogin()
{
LoginDialog dlg = new LoginDialog();
dlg.ShowModal();

if( validateUsername(dlg.GetUser()) && validatePassword(dlg.GetPwd()) )
{
// TODO: login to system
}
}

string username;
string password;
}

This time, the code reviewer notices a flaw: what if username or password validation fails? Where's the error handling?

Instead of going back and implementing the error handling, another TODO can be added before committing to source control:

if( validateUsername(dlg.GetUser()) && validatePassword(dlg.GetPwd()) )
{
// TODO: login to system
}
else
{
// TODO: handle login error
}

The code still works and is error free, although right now all it does is bring up a dialog with no functionality. Perhaps the next patch implements the password validation and login. Perhaps later on, another patch will validate the username and even later on error handling is added, etc.

What Do I Implement Next?

The use of patches, stubs and TODO's means that your developers have to think ahead about what they're going to implement. Which is a good thing. Too often we focus on the details of an algorithm and start implementing full speed ahead, without knowing exactly what direction we're going in. Working with patches means getting your team to do a little self-planning. Like in chess, every team member needs to think a few moves ahead: they should always know what the next 2 patches will be.

Working with TODO's allows you to prioritize your work. Let's say you need a rough demo by Friday - instead of implementing the whole Login dialog, implement only the essential code for the demo, and leave things like validation and error handling for the next iteration. Consistently using the same keyword "TODO" means you can always search through the code and get a feeling for how much code is left to be written, what error handling and special cases are not yet handled, etc. Getting a TODO count gives your lead a good idea of how close to completing the project.

What about once I submit my patch and am waiting for review. How do I continue working on the code? The nice thing about having patches as files is that they can be created and code reviewed in parallel. In the above example, I might write a patch for validatePassword() function, and another for validateUsername() function while I am waiting for the first to be reviewed. I might submit patches for another class I'm working on besides Login.


Bottom line: the process encourages good design, planning and decomposition of tasks from everyone, not just the project leader or manager. Once team members get good at planning ahead and breaking down their work, they might have 4-5 patches being reviewed on the mailing list all at once.

Patch-a-day is not part of a waterfall or agile methodology necessarily - it is a development process that could be implemented in any methodology to:
* improve organization of development team's work
* facilitate fast, thorough and frequent code review
* encourage structured design, decomposition of classes and functions
* prioritize implementation of critical functionality
* track remaining work including error handling and special cases

For more information, drop me a line. It'll be worth your time.