|
|
CIDev wrote: defensive offensive coding
FTFY
|
|
|
|
|
Andrei Straut wrote: Counting the while steps maybe That would be my guess. I have used counters when debugging code to count the number of times I went through a loop and append that number to a string so I could track the progress. It made it easier to find the offending record.
The only thing I can think of is that it was suggested to this particular developer to use a counter for this reason but the developer didn't understand why they were being told to do this.
It was broke, so I fixed it.
|
|
|
|
|
Hi all,
I was already reading this forum when it was still called the "Hall of shame".
I've got a product to maintain that was created by a developer who is no longer working for us.
Even though I've already seen plenty of "hall of shame worthy" code there, I've have never posted it here, because it would have been my first post on CP ever and I thought it is a bad idea to say hello to a community while bashing an ex colleague.
However, I came across a piece of code that I can't keep for myself and so I'll do what I didn't want to do and bash in my first post:
At the end of a long method I found this one:
try
{
return true;
}
catch (Exception)
{
return false;
}
Cheers,
cmger
|
|
|
|
|
Welcome to posting on CP.
I hope that there used to be more code in the try catch that would give it a reason to be there. Even if there used to be code that justified the try, when that code was removed the try/catch should have also been removed.
This is another fine example of code that might work, but really isn't good code, hence my sig.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Thanks for the welcome!
Nope, there was never anything more in there. At least as far as I can tell from the source control system.
I found out that there is only one place where the method is used and there the return value is just ignored. I re-factored the method to return void an removed the block completely. Now there is only the code that preceded the try catch.
|
|
|
|
|
Maybe it was some kind of skeleton for logic to be inserted in the try-catch block later? However, it should've been preceded by a comment at least.
cmger wrote: I found out that there is only one place where the method is used and there the return value is just ignored.
Rejoice, for you got away easily! It could've been a lot worse
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
I am not surprised that there was never any useful code there, just disappointed.
I really hate it when programmers return a value that serves no purpose.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Oh, then you would really love the code base where the above stems from,...
Not!
|
|
|
|
|
cmger Wrote: Oh, then you would really love the code base where the above stems from,...
Not!
Sounds like you have a lot more potential entries for this forum.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
Looks more like a stub - that was never filled out.
|
|
|
|
|
My guess is that this was "skeleton" code that the coder copy pasted but forgot to fill.
|
|
|
|
|
Just saw this posted on another forum
List<char> _chars = new List<char>();
for (int i = 0; i < 2000; i++)
{
if ((i >= 48 && i <= 57) || (i >= 97 && i <= 122))
{
_chars.Add((char)i);
}
}
The author claimed that the code "work efficiently". I wonder how the upper limit of 2000 was determined? Why not int.MaxValue? Then you could maximise the inefficiency. Of course, you could do better with a long, or a lower bound of int.MinValue but that's getting silly.
The way _chars was used later all that was required was
string _chars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
|
|
|
|
|
Reminds me of the old classic QA response "My code is correct, but it's not working"
|
|
|
|
|
Char.IsLetterOrDigit might have been a good starting point. Could have wasted hours of the authors time testing to see which was the more efficient
Interested though if it was also a bug, 97 to 122 are all lowercase, not uppercase.
"You get that on the big jobs."
|
|
|
|
|
Well obviously it is there "for future expansion".
Of the alphabet, I assume...
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
¥ɛѦ, tĥѦτ ѡĭ└└ ɳɛvɛr ĥѦῤῤɛɳ‼
|
|
|
|
|
I'm gonna need a bigger keyboard...
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
All you need is the 'Alt' key and a 10-key pad.
Alt-1...
☺
|
|
|
|
|
I wonder who decided that the smiley was most important key to put on Alt?
I noticed they had a Alt+2 for the other coders.
☻
|
|
|
|
|
If it will not happen then select the code file press shift + delete..
.......If possible..
|
|
|
|
|
That code might work, but I would not call it efficient.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I think you meant:
string _chars = "0123456789abcdefghijklmnopqrstuvwxyz";
(lowercase) 65 = A, 97 = a
Your version is definitely more readable (and efficient). Based on the default List length, it might reallocate (and copy) the memory backing the list a few times.
|
|
|
|
|
<td align="center" valign="middle">
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<br />
<asp:Label ID="lblError" ForeColor="Red" Font-Bold="true" Text="please contact your System Administrator." runat="server" CssClass="errortext"></asp:Label>
</td>
Regards,
Hiren.
-"I don't know, I don't care, and it doesn't make any difference".
|
|
|
|
|
Error text said: Linebreak overflow. Please contact your System Administrator.
Although, I must admit I'm I have been guilty of doing the same thing (NBSP's too), before I learned what CSS is (and the example above isn't even that elephanty, compared to the horrors I've created, and I could even go to the point of calling it ok-ish). But then again, those were the college days. I (like to believe I) suck a lot less now.
And at least the one who wrote this at least seems to know what a CSS class is, or is it just the code gen?
|
|
|
|