[From VbTeachesBadHabits]
"
WithBlocks" are one of the
CodeSmells you should watch for and eliminate.
Why not use it (cons):
- It hides other versions of the name in outer scopes, thereby introducing bugs that can be hard to find (do any compilers warn about this?)
- Potentially violates information hiding: why are you talking about some other classes instance variables in the first place? (See LawOfDemeter immediately below) And if you're talking to your own class, you don't need "with".
- It masks the smell of FeatureEnvy.
Why use to it (pros):
Previously stated but incorrect reasons why to avoid them:
- Violates the LawOfDemeter
- No, because as that page says, the whole subject of what "LawOfDemeter" truly means is confused. At the very least, do not cite that page without fixing it first.
- is PrematureOptimization (a heavy handed equivalent to ReplaceQueryWithTemp)
- No. The point is abbreviation for readability, not speed optimization. With current compilers for VB/Pascal etc, there is no speed difference.
- is just SyntacticSugar
- That's all that higher level languages are in the first place, "just SyntacticSugar". That's an empty argument.
- LambdaExpressions, HighOrderFunctions, Inheritance, Polymorphism, Encapsulation, LazyEvaluation are all SyntacticSugar?
- Of course. All programming languages are syntactic sugar above the language one level below, all the way down to assembly language being syntactic sugar for binary machine code
- (Agreed, it's just syntactic sugar all the way down!)
- (Until you get to the Turtles...)
- (Mmmm.... sugar-coated turtles....)
Alternatives:
- ExtractMethod: Make the WITH block a function or method. Really, it should be a method on the object named in the WITH clause, so after extracting the method, consider applying the patterns MoveMethod, ForeignMethod, or IntroduceLocalExtension.
- Refactor the passive data structure/record, making it an object -- where you can put the method (ExtractMethod).
- Encapsulate the passive data structure/record or unchangeable 3rd party object in a wrapper object -- where you can put the method (ExtractMethod).
On the other hand...
- It may be useful when setting large amounts of attributes on a single object, as often seems to be needed with GUI components.
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 String
Builder (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