Today, for about the three millionth time in my career, I ran across this bit of code:
string someVariable = someObject.SomeProperty;
DoSomethingWith(someVariable);
Given that “someVariable” is never used again, what in the name of all that is holy, is wrong with:
DoSomethingWith(someObject.SomeProperty);
I realize it’s a small thing, but in my experience small things add up. Maybe in today’s world it doesn’t matter in terms of resources that you’ve wasted a few bytes by leaving a needless copy of a string to float around in memory for the duration of the method. But if you do this here, you’re going to do it everywhere else, including in places where it matters a great deal. And I’m not just talking memory consumption or performance, either. Generally speaking, there’s twice as much to go wrong in two lines of code as in one.
When you think of it, the one-line version of the above succinctly expresses what you should be thinking: “I need to do something with some property of this object over here”. The two-line version says: “I need to get the value of some property of this object over here, and then do something with it.” Yes, that is technically accurate also, but it’s also a needless bifurcation.
Do you think that keyboard virtuosos laboriously figure out each of the notes in a musical chord? No, they look at it and recognize it instantly as, say, the 2nd inversion of an F-major chord, and their fingers instantly know where to go. If they didn’t, no song would ever exceed the tempo of a funeral dirge and all of them would be about as expressive as a tub of mud.
In my view you should never create a variable for one-time use unless the resulting code would be significantly more readable as a result — for example, if you need to pass three or four arguments to a method and all of them are lengthy references or expressions, maybe then it’d make some sense from a readability standpoint to pull the values into variables first.
I suspect that readability would be the excuse for the needless assignment in my example above, but the truth is, the real reason would be habit and there is no real purpose behind it. And I’d make the argument that the one-line version is more readable and self-evident.
Clean, elegant, mindful code is created one good habit at a time. Don’t leave tiny little messes like this one behind for others to either cope with or clean up. It’s death by a thousand cuts.
{ 1 comment… read it below or add one }
This used to make sense when the string declaration was just a pointer and no data was being copied and the dereference to the object property was more time consuming than the pointer reference. Moral: know what your code is doing with memory.
Bob responds: As Daffy Duck once said, “Hmmm….. pronoun trouble!” What, Charles, is “This” referring to? I frankly can’t figure out if you agree or disagree with my comments (moral: know what your prose is doing with communication). But I will say this about “knowing what your code is doing with memory” … my objection is to creating a variable that will be referenced exactly one time. If one is going to reference it once in a loop that will be executed a thousand times, then there is a different dynamic; quite possibly dereferencing the property once will be less expensive (assuming no compiler optimizations in this regard), and the code may be slightly more readable as well. I would submit however that most of the time the difference will not be enough to be worth the effort to benchmark it, and on those occasions where it’s marginally worth it, it’s STILL not worth it because a given user’s mileage will vary according to their system resources and the pressures on those resources at any given moment.
My personal bent is to write self-evident code, with a few basic performance principles like avoiding loop-invariant code, but not really obsessing about performance and resource consumption unless there is a demonstrated problem in those areas that you can empirically trace to its actual source. Really my biggest issue with the example in this post is that it needlessly exposes two sub-steps within a single conceptual step, and then leaves a useless variable hanging around for the duration. It’s more a matter of clarity, housekeeping and good order than of performance or resource management. The latter are arguably more important in the gaming world (for example) but for line of business applications — feh!