One thing I've noticed of the past few years is the amount
of developers/programmers/ninjas that do take the easy and quick way to do
certain tasks.
With each release of .NET things get easier for us. Things are bundled
together, we have to do fewer checks, and it's all nice. But, as much as we're
being spoiled by all these cool encapsulated functions, we're forgetting some
very basic coding practices.
C# has garbage collection, so why should we have to destroy objects? There are
timeouts on SQL Connections, so why should we close DataReaders? Servers these
days are so powerful it's ok if I make every ASP.NET object global... it makes
my code easier...
What's happened to us? Now some of you might think: "That can't be so
Ryan. No one would do that." Unfortunately it's true. With many new
developers hitting the main stage this is becoming more and more of an epidemic.
Over the next few weeks I have been helping many people on
the asp.net forums, as well as through my blog/email. I’ve had to show people
fundamental programming standards (drilled in me in the C/C++/Cobol days) that
they had no idea about.
Over the next few weeks I will be doing posts on doing
things the quick way, and doing things the proper way. Proper usually entails “More
Code”, but “more code” can lead to quicker processing, less memory usage, and
an overall satisfaction of doing something “right” and not “quick” (for those
that are married this implies to more than just code).
Looping.
Every time we get an outside source to do a bit of
consulting for us or we outsource some code, or we buy some code I like to look
at it. I like to see how others coded certain parts of the tools we use to see
if I can learn anything. While looking
at one piece of code I saw this:
foreach (int i in
aintColumns)
for
(int j = 0; j < lstFields.Items.Count; j++)
if
(lstFields.Items[j].Value == i.ToString())
{
lstFields.Items[j].Selected = true;
}
Immediately I cringed. To give some background, lstFields is
a ListBox with MultipleSelect set to true. The aintColumns is an int[] that
holds the Values that need to be selected in the lstFields ListBox.
This snipped is actually what needs to be done, but they’re missing one
specific word… BREAK! What this current piece
of code will do is: “Oh look, I found the right value… let’s continue on
through the loop to see if I find any more”.
The problem is, there will not be any more, and if there are… it will
still be selected, so it’s redundant, sloppy and wastes processing. Yea, in the grand scheme of things it might
be hardly any extra work… if the lstFields only has like 10 items… but what if
it had 100? And 10,000 people decided to use this control at the same time.
Those extra loops could be your worse nightmare.
So how do we fix it? Add a break like this:
foreach (int i in
aintColumns)
for
(int j = 0; j < lstFields.Items.Count; j++)
if
(lstFields.Items[j].Value == i.ToString())
{
lstFields.Items[j].Selected = true;
break;
}
Not to be confused with continue, break will break the execution
of the loop it’s contained in and continue processing the code – which in this
case will just go to the next i in aintColumns. Continue would continue the
loop.