The Weird and The Wonderful

That's cute, set up values, don't use them and then blow up when the if test passes and rgb[1] == "G" :laugh: Well, if it does return true, you know you "CanParse" rgb[0] :) There are a lot of things I see in code that don't make sense. Interesting, "int i = 350; byte b = (byte)i;" works fine (sorta), but if you byte.TryParse(i.ToString(), out b); it will return false while b changes from 94 to 0.

Whehe, aight :-D The last example shows how it does more damage than good. I've seen these quite often, and it costs more time than that it actually helps. Was I right in my guess? And if I also guess he's Dutch? Bastard Programmer from Hell :suss:

Vitaly Tomilov wrote: Looks like a simple error, which needs a simple fix. Firstly, simple error yes, but why is this not spotted a mile away? Secondly, this is not my code. It is Microsoft's production code, that lives in VS2010 and VS2012. It is up to them to fix such a simple error. IronScheme ((λ (x) `(,x ',x)) '(λ (x) `(,x ',x)))

:thumbsup: ------------------------------ Author of Primary ROleplaying SysTem How do I take my coffee? Black as midnight on a moonless night. War doesn't determine who's right. War determines who's left.

That reminded me of a custom exception one of my friends made: NoobDetectedException. He said he would take it out when he turned in the project, but he didn't. The professor hit the exception, and was not very pleased. The professor showed the class the exception, which had the message: "You are a noob. You will be assimilated. There will be no escape. (This also applies to <professor name omitted>") public class SysAdmin : Employee { public override void DoWork(IWorkItem workItem) { if (workItem.User.Type == UserType.NoLearn){ throw new NoIWillNotFixYourComputerException(new Luser(workItem.User)); }else{ base.DoWork(workItem); } } }

Perhaps I was not clear. You summarized my exact thoughts

I'm the guy that fix code :'( jajajaja

Being a bit of a devils' advocate here. I think this code definitely require a better message in the exception or description in the code. However Perhaps the programmer want to prevent the class from being inherited. This is why he made it internal. He could have make it public and sealed and as I see it get the same effect. Sometime when you define an api though, it might not be such a bad idea to keep everything in interfaces. This way the library can be modified without impacting the Interfaces that have been exposed previously.

Still worse: when the array list does not contain "something", the default is set to something. When the array list contains something, the default is set to the first element of the array list, which may or may not be "something"...

Agreed - variable masking is a bad idea, it leads to some very hard to spot bugs. Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water

I'd be looking for other related error messages: MessageBox.Show("Danger Will Robinson"); MessageBox.Show("Never fear, Smith is here!");

Yep, SOP is to provide little time, little money, little information, then blame the programmers when the software isn't perfect. :sigh: Just because the code works, it doesn't mean that it is good code.

IMO it's a beginner programmer's mistake - anybody who has ever done a web app has learnt this lesson the same way. It isn't often that testers find this type of error - usually programmers, or whatever library or framework they use take care of this.

Then I would amend it to: size = 29; if(width > 11 || height > 4) size++; if(width > 13 || height > 5) size++; if(width > 15 || height > 6) size++; if(width > 18 || height > 7) size++; if(width > 19 || height > 8) size++; if(width > 21 || height > 10) size++; switch(size) { case 29: Spread(width, 15, 16, xpos, ypos); break; case 30: Spread(width, 13, 14, xpos, ypos); break; case 31: Spread(width, 11, 12, xpos, ypos); break; case 32: Spread(width, 9, 10, xpos, ypos); break; case 33: Spread(width, 9, 9, xpos, ypos); break; case 34: Spread(width, 8, 7, xpos, ypos); break; case 35: SpreadCompressed(width, width > 21, height > 10, xpos, ypos); size--; //back to 34 for you break;

Zac Greve wrote: #if DEBUG //DEBUG CODE #else //RELEASE CODE #endif True, and I do that a lot. Though it's bit me a couple times that I now watch out for... a) In most situations I'd really rather have 3 levels - DEBUG on my dev machine, DEBUG on the dev server, RELEASE on the production server. b) Release code paths don't get tested as thoroughly until uploaded to the production server. I mean - you have to test them, but if you wrap everything in #if DEBUG directives, it can be nontrivial to run in VS on a dev system in RELEASE mode.

Try working on a limited stack-depth platform, such as the PIC16F1516 - your tune will quickly change when you hit the stack depth with your precious function calls. Without goto on that platform you'd be royally f***ed for anything more than the simple. Tail recursion on a PIC: GOTO! The worst thing about the darkness is the light at the end - DX-MON

I remember investigating one application that tried to jump to an invalid location. That app was innocent. Some other code used COMMON to reach what happened to be that program's current memory location, re-wrote the computer instructions including the said jump statement. The app was just running it's instructions when all of a sudden it wasn't their instruction set anymore.

My sympathies to you for having to put up with that nonsense. Just because the code works, it doesn't mean that it is good code.

I think his & key was not working other wise he would have used a && to do it. poor guy.

Rahul Rajat Singh wrote: Little confused. why is this in "Hall of shame"? Naerling may be right, but maybe the original OP was ashamed he didn't understand the code and didn't understand the point of this forum.