Anti-Patterns – What Is the For-Case Antipattern?

This is an antipattern as it takes the form of:

loop over a set of values
   if current value meets a condition
       do something with value
   end
end

and can be replaced with

do something with value

A classic example of this is code like:

for (var i=0; i < 5; i++)
{
    switch (i)
        case 1:
            doSomethingWith(1);
            break;
        case 2:
            doSomethingWith(2);
            break;
        case 3:
            doSomethingWith(4);
            break;
        case 4:
            doSomethingWith(4);
            break;
    }
}

When the following works just fine:

doSomethingWith(1);
doSomethingWith(2);
doSomethingWith(3);
doSomethingWith(4);

If you find yourself performing a loop and an if or switch, then stop and think about what you are doing. Are you overcomplicating things and can the whole loop and test just be replaced with a simple "just do it" line. Sometimes though, you will find you need to do that loop (more than one item might match the one condition, for example), in which case the pattern is fine.

That's why it's an anti-pattern: it takes the "loop and test" pattern and abuses it.

Regarding your second question: yes. A "try do" pattern is more robust than a "test then do" pattern in any situation where your code isn't the sole thread on the whole device that can change the state of the item under test.

The problem with this code:

if (File.Exists("desktop.ini"))
{
    return new StreamReader("desktop.ini");
}

is that in the time between the File.Exists and StreamReader attempting to open that file, another thread or process could delete the file. So you'll get an exception. Therefore that exception needs to be guarded against via something like:

try
{
    return new StreamReader("desktop.ini");
}
catch (File­Not­Found­Exception)
{
    return null; // or whatever
}

@Flater raises a good point? Is this itself an antipattern? Are we using exceptions as control flow?

If the code read something like:

try
{
    if (!File.Exists("desktop.ini")
    {
        throw new IniFileMissingException();
        return new StreamReader("desktop.ini");
    }
}
catch (IniFileMissingException)
{
    return null;
}

then I would indeed be using exceptions as a glorified goto and it would indeed be an anti-pattern. But in this case we are just working around the undesirable behaviour of creating a new stream, so this isn't an example of that anti-pattern. But it is an example of working around that anti-pattern.

Of course, what we really want is a more elegant way of creating the stream. Something like:

return TryCreateStream("desktop.ini", out var stream) ? stream : null;

And I'd recommend wrapping that try catch code in a utility method like this if you find yourself using this code a lot.

Some suggestions:

• There is nothing wrong in having a lot of feature or bugfix branches as long as the changes done in each branch are small enough you can still handle the resulting merge conflicts in an effective manner. That should be your criterion if your way of working is ok, not some MSDN article.

• Whenever a branch is merged into trunk, the trunk should be merged into all open development branches ASAP. This would allow all people in the team to resolve merge conflicts in parallel in their own branch and so take some burden from the gatekeeper of the trunk.

• This would work way better if the gatekeeper would not wait until 10 branches are "ready for merging into trunk" - resolving merge conflicts from the last trunk integrations always needs some time for the team, so it is probably better to work in interwoven time intervals - one integration by the gatekeeper, one re-merge by the team, next integration by the gatekeeper, next re-merge by the team, and so on.

• To keep branches small, it might help to split larger features into several smaller tasks and develop each of those tasks in a branch of its own. If the feature is not production ready until all subtasks are implemented, hide it from production behind a feature toggle until all subtasks are completed.

• Sooner or later you will encounter refactoring tasks which affect many files in the code base - these have a high risk of causing a lot merge conflicts with many branches. Those can be handled best by communicating them clearly in the team, and make sure to handle them exactly as I wrote above: by integrating them first into all dev branches before reintegration, and by splitting them up into smaller sub-refactorings.

• For your current team size, having a single gatekeeper may still work. But if your team will grow in size, there is no way around having a second gatekeeper (or more). Note I am not suggesting to allow everyone to merge into trunk, but that does not mean only your boss is capable of doing this. There are probably one or two senior devs who could be candidates for doing the gatekeeper's job, too. And even for your current team's size, a second gatekeeper could make it easier for your team to integrate to the trunk more often and earlier, or when your boss is not available.