When do you know you're dealing with an anti-p

问题:

I'm looking to see how other programmers find anti-patterns, "code-smells", etc.

In particular, what things start setting you off when you're looking at code that tells you, something has gone wrong here?

I'm not looking for a list of different patterns like this post. I want to know how you've found such things in code bases you've worked on in the past.

回答1:

Actually, the first and most clear warning sign that there is something severely wrong is not at all in the code; it's in the organization. Organizations that display clearly toxic or bizarre traits tend to have codebases that show antipatterns. It's uncertain as to whether the codebase being messed up is the result of the antipattern, or the organization being messed up is the result of the codebase being messed up, but the upshot is that the easiest way to notice an antipattern is to see something going horribly wrong in the organization.

Sometimes, it can be a while before you get deep enough into the codebase to determine that the codebase is complete spaghetti; usually before that, though, you'll find a coworker who explains with a wry comment that "you don't want to open Pandora's box by trying to change anything". Long work hours can be another indicator of antipatterns at work; so can "guru syndrome" ("Bob's the guru; he's the only one who understands the system"). These things all show up as organizational problems far before you end up seeing them in code.

Since there seems to be a desire to get some metrics as to what a dysfunctional organization is, here are a few indicators (by no means exhaustive):

• Massively disgruntled workers
• Timekeeping obsessed management
• Shoehorning ("Golden hammer" antipattern)
• High turnover rate among engineers
• Bad ratio of dev / test (either too high or too low is an indicator)

These aren't really metrics; they're just really warning signs. It really comes down to personal experience to know what constitutes "massively disgruntled" versus "mildly disgruntled", etc.

回答2:

Here's the giveaways:

• Lots of side effects
• Lots of regressive bugs
• Very difficult to test
• Doesn't fit in with other libraries
• Difficult to refactor
• Difficult to extend.

Come to think of it, this is pretty much any bad code.

回答3:

One of the things I notice fairly quickly is the amount of historical comments and commented out code. This is typical of developers that don't use or don't trust source control.

回答4:

I ran the Eclipse Metrics plugin.

When you have many, MANY methods with cyclomatic complexity scores of well over 20 (one was 66...) and regularly see methods with dozens of parameters or hundreds of lines of code, you know something has gone wrong.

It doesn't tell you everything (metrics never do), but when you start seeing numbers like that, it gives you a place to start looking.

回答5:

Anti-patterns are identifiable the same way patterns are; they're both descriptions of a general technique that "smells" the same no matter where you find it. So when you see a class that imports most of the available namespace, has 30 000 lines, and has more commits than the rest of the codebase combined, chances are you're looking at a God Object.

The important thing to note, I think, is that (as McWafflestix and Christopher point out) bad code is like obscenity. You know it when you see it.

回答6:

-Whenever asking about a section of code/module produces a response of "Oh, XXX did that."

-Whenever changing a section of code produces a reaction of "Yikes, I can't believe you changed that."

回答7:

McWafflestix has a lot of good points at the organizational level. A dysfunctional software engineering organization will make it nearly impossible to produce good code, if only for simple pragmatic reasons such as lacking decent tools and automation.

Even with a functional organization and good tools, however, anti-patterns can grow on you. Some characteristics I look for are below, with the rules of thumb I use in parentheses.

Some signs:

• In OOP, extremely deep object hierarchies (more than 3 levels)
• Copy-and-paste code that appears multiple times (more than 3) OR code in multiple locations that performs substantially the same function
• Copy-and-paste code with minor/subtle changes that cannot be easily/sensibly parameterized

I also tend to look for areas in the code which are not intuitive, readable, (insert favorite adjective here). This speaks less to code quality and more to your team's ability to work together effectively. Over time, a team's codebase usually picks up a dialect - common semantics and vocabulary for method and variable names, informal conventions about ordering methods or operations, etc.

Ensuring the consistency of that dialect can help significantly, especially when one team member has to work on a segment of code that had, until that moment, been maintained by another person. When code begins drifting substantially enough off the common dialect, that can be a sign that a design anti-pattern is emerging.

回答8:

When the comments have the word "trick" in them, as in "A trick for figuring out which customer to return when we don't know which one we are supposed to use.

Seriously, I think it's like obscenity: You'll know it when you see (smell) it.

• code with lots of bugs logged against it, but the code itself has not been updated in 3 years or more.
• database structures with comma-delimited values within a single column
• code with no comment, no meaningful names, and and lots of numbers
• code where there's only one guy or gal who is allowed to maintain it.

回答9:

I think it pretty much boils down to experience and intuition.

回答10:

There are things that good design gives you, and bad designs don't. When you can't extend it without significant rewriting, when there are lots of "clever ideas" but the code is hard to work with, when it's brittle, etc...then it smells. Note that anti-patterns are a subset of smelly code - see:

http://en.wikipedia.org/wiki/Anti-pattern "there must be at least two key elements present to formally distinguish an actual anti-pattern from a simple bad habit, bad practice, or bad idea:

Some repeated pattern of action, process or structure that initially appears to be beneficial, but ultimately produces more bad consequences than beneficial results, and

A refactored solution that is clearly documented, proven in actual practice and repeatable."

回答11:

My personal pet peeve in this regard is a large number of "dummy" or "mockup" forms. Wherein someone creates a mockup of the form, then creates a new mockup when the requirements change, then creates a third mockup when they stabilize, and then, finally, makes the real form. At the end, you wind up with file names like

-frmAddUserWizard
-frmAddUserWizardv2
-frmAddUserWizard_MOCKUP
-frmAddUserWizard_OLD

And ALL of these forms are accessible from the application, because we need to display them to take a screenshot to show the user! At a glance, its impossible to tell which of the forms is the real one (tip: in the above example, its the second entry). But nobody wants to delete the old code, because they represent work that they put in, and they don't want to part with it.

edit: This can also happen when forms get re-used in the application, and "copypasta" inheritence is used.

回答12:

Sorry to add to this late - I just wanted to contribute that in my experience a lot of cruft in the main codebase seems to indicate that no-one has really cared for it. For instance you have classes that seem to do something but you find they have been superseded by other development and are never called, having investigated at length.

回答13:

Conflicting Styles.

When you read through the code, you quickly figure out who wrote it- Joe always uses state machines, John always uses event driven callbacks, and Fred is Mr Two-line-function.

If their style reflects the needs of that part of the program, great, but when it reflects dueling architectural ideas, with no one in particular winning, then you know the code really smells.