With Block Code Smell

[From VbTeachesBadHabits]

"WithBlocks" are one of the CodeSmells you should watch for and eliminate.

Why not use it (cons): Why use to it (pros):

Previously stated but incorrect reasons why to avoid them: Alternatives: On the other hand...
In a proper OO-language, wouldn't the need for a WithBlock violate the LawOfDemeter? Instead of declaring a WithBlock and manipulating the object, why not simply ask the object to do this for you?

WithBlocks are essential for data-oriented programming, where you have a record and you want to do stuff with it. In an OO language (especially as VB is trying to recast itself), they are unneeded (or worse, harmful). -- RobertWatkins

WITH is never "needed." Like just about any programming language construct, it's there for convenience and clarity of exposition. And while you can use WITH to violate the LawOfDemeter, using it doesn't automatically make you a felon. If your object has at least two methods, fields or properties, or if you want to call a method more than once (with different parameters, perhaps), then you can use WITH, and you wouldn't be violating the LawOfDemeter. -ss
Yes... Using with is a bit like applying the MoveMethod refactoring and stopping half-way.
[from C++ example on WithBlocks page]

"It just comes out with the wash if you write short functions." [In other words, the WithBlocks go away.]

If you have functions with locally scoped argument names, you probably don't need WITH. The desire to have a shorter name for an expression, just for a few lines of code, is a CodeSmell that says those lines want to be a function. As RobertWatkins says, the function probably belongs in the Document class (in which case we can drop the "d" altogether).

[See WithBlocks page for C++ example with the code block containing the line "Document *d = activeDocument;"]
[from WithBlocks]

I'd say such a structure would be useful when setting large amounts of attributes on a single object, as often seems to be needed with GUI components.

On the other hand, if you're setting lots of properties or calling lots of methods on another object, maybe one should ExtractMethod and put the new method on the other object (or a wrapper, if you can't change the other object).

I'm a mainly BorlandDelphi programmer, which unfortunately has with blocks, whenever I see lots of them the alarm bells start ringing. They tend to be used by the sort of programmers who write just about all their code inside GUI event handlers, rather than using domain classes.

-- SteveEyles

And just what is wrong with putting code in event handlers? Is this related to SeparateDomainFromPresentation? --top


Why is the VB with statement considered more of a concern than the Java import statement? Both seem to accomplish the same thing, but the VB with statement provides more restrictive scoping.

The VB With statement (VB preferred style uses PascalCase keywords) is different from the BorlandDelphi (or other dialects of Pascal) with statement in that it doesn't override scoping. Inside a With X block, X.Foo is accessed as .Foo which doesn't shadow a local or member Foo. In any case, With is nice when frobbing the quux out of a GUI widget, throwing a pile of stuff into a StringBuilder (the CLR's equivalent of Java's StringBuffer), implementing a copy constructor, deconstructing a result set, or otherwise transferring quantities of often dissimilarly-formatted state between objects. One-character variable names are a scope-polluting substitute.
C# has CodeSmell form an absence of With blocks. How annoying is it to constantly horizontally scroll when examining code like this:

MyExtremelyDescriptiveClassInstance?.Attribute1 = .... MyExtremelyDescriptiveClassInstance?.Attribute2 = .... ... MyExtremelyDescriptiveClassInstance?.AttributeX = ....

instead of:

With MyExtremelyDescriptiveClassInstance?
  .Attribute1 = ...
  .Attribute2 = ...
  ...
  .AttributeX = ...
End With

I've seen literally 30+ lines of such code, such as when composing a transactioned SQL string with stringbuilder. Horizontal scrolling hampers readability, understandability and general code maintenance. This is where some OOP languages become hypocritical. The whole point is to factor out duplicate code, and copy/pasting the same concept over and over for several lines is CodeSmell

IMO "with" here is the code-smell analog of spraying air freshener while ignoring the problem. Refactor! What are you doing composing SQL with a StringBuilder anyway? Get to the core of the problem, and deal with it, rather than merely hiding it.

It's called using.
   { using System;
      // stuff in the namespace system.
   }
   using s = System {
       // do stuff with s.
   }

No, using is for importing type names from other namespaces. The issue at hand is setting lots of properties on a single object (instance). But it's worth noting that this is largely solved in C# 3.0 with object initializers.
    var a = new Foo { 
        Prop1 = 42, 
        Prop2 = 84
    };

CategoryCodeSmell

EditText of this page (last edited September 3, 2013) or FindPage with title or text search