An anti-pattern I've seen too many times is using display text to drive logic. For example, I've see things like:

void btnClick(Object sender, EventArgs evt) {
    if (((Button)sender).Text.Contains("Done")) {
        …
    }
}

That is, of course, bad on its face, an awful sign of other WTFs which may be lurking, and an instant "nope". Today's anonymous submitter inherited some code from an outsourcing company, and they "gracefully" avoided this anti-pattern.

private bool IsDone()
{
        if(labelNext.DatabaseKey == "Common.Done")
        {
                return true;
        }
        return false;
}

Unlike the obviously terrible version I pseudocoded above, this version at least supports internationalization. In this case, the DatabaseKey is a tag that can be used to look up a localized text string for the displayed label. So while this is marginally better, it's still driving program logic by looking at what's being displayed by the UI, instead of having the display driven by program logic.

Of course, I'm mostly annoyed because IsDone, even as implemented, could be a one-liner: return labelNext.DatabaseKey == "Common.Done".

Fortunately for our submitter, there's a _done field that does handle tracking application state. So it was a trivial exercise for our submitter to replace the implementation with:

private bool IsDone()
{
        return _done;
}
[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!