[Carbon-dev] [Code review] Code Smell

Afkham Azeez azeez at wso2.com
Sun Apr 25 23:35:38 PDT 2010


Smell to Refactoring is a very useful 2 page document which
provides guidelines on how to refactor the code when you detect a code
smell. See http://industriallogic.com/papers/smellstorefactorings.pdf

Azeez

On Mon, Apr 26, 2010 at 11:45 AM, Afkham Azeez <azeez at wso2.com> wrote:

> Please read http://www.codinghorror.com/blog/2006/05/code-smells.html.
> Also look at some of the important links in that article.
>
>
> Developing your "code nose" is something that happens early in your
> programming career, if it's going to happen at all. I combined all the
> documented code smells I could find into this reference; most of these
> smells should be familiar to you.
>
> *Code Smells Within Classes*
> CommentsThere's a fine line between comments that illuminate and comments
> that obscure. Are the comments necessary? Do they explain "why" and not
> "what"? Can you refactor the code so the comments aren't required? And
> remember, you're writing comments for people, not machines. Long MethodAll
> other things being equal, a shorter method is easier to read, easier to
> understand, and easier to troubleshoot. Refactor long methods into smaller
> methods if you can. Long Parameter ListThe more parameters a method has,
> the more complex it is. Limit the number of parameters you need in a given
> method, or use an object to combine the parameters. Duplicated codeDuplicated
> code is the bane of software development. Stamp out duplication whenever
> possible. You should always be on the lookout for more subtle cases of
> near-duplication, too. Don't Repeat Yourself!<http://www.artima.com/intv/dry.html> Conditional
> ComplexityWatch out for large conditional logic blocks, particularly
> blocks that tend to grow larger or change significantly over time. Consider
> alternative object-oriented approaches such as decorator, strategy, or
> state. Combinitorial ExplosionYou have lots of code that does *almost* the
> same thing.. but with tiny variations in data or behavior. This can be
> difficult to refactor-- perhaps using generics or an interpreter? Large
> ClassLarge classes, like long methods, are difficult to read, understand,
> and troubleshoot. Does the class contain too many responsibilities? Can the
> large class be restructured or broken into smaller classes? Type Embedded
> in NameAvoid placing types in method names; it's not only redundant, but
> it forces you to change the name if the type changes. Uncommunicative NameDoes
> the name of the method succinctly describe what that method does? Could you
> read the method's name to another developer and have them explain to you
> what it does? If not, rename it or rewrite it. Inconsistent NamesPick a
> set of standard terminology and stick to it throughout your methods. For
> example, if you have Open(), you should probably have Close(). Dead CodeRuthlessly
> delete code that isn't being used. That's why we have source control
> systems! Speculative GeneralityWrite code to solve today's problems, and
> worry about tomorrow's problems when they actually materialize. Everyone
> loses in the "what if.." school of design. You (Probably) Aren't Gonna
> Need It <http://xp.c2.com/YouArentGonnaNeedIt.html>. Oddball SolutionThere
> should only be one way of solving the same problem in your code. If you find
> an oddball solution, it could be a case of poorly duplicated code-- or it
> could be an argument for the adapter model, if you really need multiple
> solutions to the same problem. Temporary FieldWatch out for objects that
> contain a lot of optional or unnecessary fields. If you're passing an object
> as a parameter to a method, make sure that you're using all of it and not
> cherry-picking single fields.
> *Code Smells Between Classes*
>
> Alternative Classes with Different Interfaces If two classes are similar
> on the inside, but different on the outside, perhaps they can be modified to
> share a common interface.Primitive Obsession Don't use a gaggle of
> primitive data type variables as a poor man's substitute for a class. If
> your data type is sufficiently complex, write a class to represent it. Data ClassAvoid
> classes that passively store data. Classes should contain data*and* methods
> to operate on that data, too.Data Clumps If you always see the same data
> hanging around together, maybe it belongs together. Consider rolling the
> related data up into a larger class. Refused BequestIf you inherit from a
> class, but never use any of the inherited functionality, should you really
> be using inheritance? Inappropriate IntimacyWatch out for classes that
> spend too much time together, or classes that interface in inappropriate
> ways. Classes should know as little as possible about each other. Indecent
> ExposureBeware of classes that unnecessarily expose their internals.
> Aggressively refactor classes to minimize their public surface. You should
> have a compelling reason for every item you make public. If you don't, hide
> it. Feature EnvyMethods that make extensive use of another class may
> belong in another class. Consider moving this method to the class it is so
> envious of. Lazy ClassClasses should pull their weight. Every additional
> class increases the complexity of a project. If you have a class that isn't
> doing enough to pay for itself, can it be collapsed or combined into another
> class? Message ChainsWatch out for long sequences of method calls or
> temporary variables to get routine data. Intermediaries are dependencies in
> disguise.  Middle ManIf a class is delegating all its work, why does it
> exist? Cut out the middleman. Beware classes that are merely wrappers over
> other classes or existing functionality in the framework. Divergent ChangeIf,
> over time, you make changes to a class that touch completely different parts
> of the class, it may contain too much unrelated functionality. Consider
> isolating the parts that changed in another class. Shotgun SurgeryIf a
> change in one class requires cascading changes in several related classes,
> consider refactoring so that the changes are limited to a single class. Parallel
> Inheritance HierarchiesEvery time you make a subclass of one class, you
> must also make a subclass of another. Consider folding the hierarchy into a
> single class. Incomplete Library ClassWe need a method that's missing from
> the library, but we're unwilling or unable to change the library to include
> the method. The method ends up tacked on to some other class. If you can't
> modify the library, consider isolating the method. Solution SprawlIf it
> takes five classes to do anything useful, you might have solution sprawl.
> Consider simplifying and consolidating your design.
>



-- 
Afkham Azeez
Software Architect & Product Manager, WSO2 WSAS; WSO2, Inc.; http://wso2.com,
Lean . Enterprise . Middleware
Member; Apache Software Foundation; http://www.apache.org/
email: azeez at wso2.com cell: +94 77 3320919
blog: http://blog.afkham.org
twitter: http://twitter.com/afkham_azeez
linked-in: http://lk.linkedin.com/in/afkhamazeez
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.wso2.org/pipermail/carbon-dev/attachments/20100426/5b6e225e/attachment-0001.htm 


More information about the Carbon-dev mailing list