|
We are dealing with application left by our former developer.
I could mention here a lot of things like putting all of the code in one file (15k lines, 680kB), putting all controls on one form and using BringToFront , naming all controls like textBox85 , hardcoding the same connection string in three different places, and so on.
But criteria if one of the buttons should be shown beats all of it:
if ((textBox39.BackColor == Color.White) && (textBox40.BackColor == Color.PapayaWhip))
button37.Visible=true;
else
button37.Visible=false;
|
|
|
|
|
Think of it this way... If it was WPF, it would be data-bound through two object models and a multi-converter to be absolutely certain that if those backgrounds (backcolors) EVER changed for any reason, the button would immediately disappear.......
.....With a psychadelic *poof* animation...
.....and background music...
.....in full DirectX-rendered 3D.
|
|
|
|
|
<Homer>
Mmmm... papaya whip...
</Homer>
|
|
|
|
|
Ar we talking whips made of papaya or papaya whipped up with cream?
Panic, Chaos, Destruction.
My work here is done.
|
|
|
|
|
Nagy Vilmos wrote: papaya whipped?
actually squashed PeachPuff
PS to Homer: even more Mmmm
Luc Pattyn [Forum Guidelines] [My Articles]
The quality and detail of your question reflects on the effectiveness of the help you are likely to get.
Show formatted code inside PRE tags, and give clear symptoms when describing a problem.
|
|
|
|
|
Brilliant!
Well, I'm a great The Simpsons fan. I truly heard Homer saying it.
|
|
|
|
|
What an idiot. Clearly he should have done.
button37.Visible = textBox39.BackColor == Color.White && textBox40.BackColor == Color.PapayaWhip;
cheers,
Chris Maunder
The Code Project Co-founder
Microsoft C++ MVP
|
|
|
|
|
You know what is worst in this situation? That, this was my first thought.
Idea of putting background color into the condition fits so perfect in his methodology, that I missed it first.
|
|
|
|
|
jkruza wrote: Idea of putting background color into the condition fits so perfect in his methodology, that I missed it first.
Doesn't everyone use background colors for their control logic?
This code probably even works, but see my tag line…
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
jkruza wrote: naming all controls like textBox85
Yikes! I cannot stand it when people do that. My course I teach, I penalize points from students who name controls in this manner, and people still do it (then they wonder why they receive a lower grade than the grade they think they were going to earn)
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
naming all controls like textBox85
I wish Microsoft had made it easy (if not default) to have VS ask the name of new objects being created, especially since the text field will get initialized to the default name (so even if the name and text field should be the same, both will have to be keyed in).
BTW, any recommendations for the best plug-in for 2005 and 2008 to fix that?
|
|
|
|
|
Looking at the type of code and namings I would hazard a guess that this might have been taken from code that was decompile using reflector etc. That would be borne out by the use of three hard coded connection strings too. It may have originally been a single constant. Not that this is an excuse or mitigates the situation. Anyone who has to rip code to that extent needs to get a new career! 
|
|
|
|
|
Even if it was decompiled, that would still mean that all the show / hide conditions are tied to various color settings. Anyone who has to rip code to that extent needs a freaking lobotomy.
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
Even if it was decompiled, that would still mean that all the show / hide conditions are tied to various color settings. Anyone who has to rip code to that extent needs a freaking lobotomy.
I've seen code like that on systems from any era that supported color (it would be rather hard to use such code on a system that didn't). Many types of controls support a 'tag' property which could be pointed to a sensible object including all necessary ancillary information, but some controls don't. If there's no need to serialize the controls, color might not be totally horrible if one used constants named based upon [i]meaning[/i] and explicitly stated in the comments that all values must be unique (I think the use of a Select Case construct somewhere could enforce that requirement). I would consider the following horrible: If ThisThing.Color = Colors.Red Then ThisThing.Delete but I would consider If ThisThing.Color = FlagColors.Deletable Then ThisThing.Delete to be rather less horrible. I would guess that both pieces of source would yield identical compiled code, however, so a disassembler would guess at the former meaning.
|
|
|
|
|
This retina scarring horror jumped out the screen and tried to garrote me with a cheese wire.
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
Just came across a class which has tens of such properties. These properties are called by other classes and by other properties / methods of this class!!! Oh! and also look how the property name says total, but is actually returning an average
public double CrappyTotal
{
double CrappyTotal = 0;
try
{
CrappyTotal = this.SomeItem.Details[0].Value;
CrappyTotal += this.SomeItem.Details[1].Value;
CrappyTotal += this.SomeItem.Details[2].Value;
CrappyTotal += this.SomeItem.Details[3].Value;
CrappyTotal = CrappyTotal / 4;
}
catch
{
CrappyTotal = -1;
}
return CrappyTotal;
}
modified on Wednesday, July 8, 2009 11:35 PM
|
|
|
|
|
|
would the path ever get into the catch block anyway? wow lol i think my eyes just burned out.
|
|
|
|
|
this.SomeItem.Details = new double[3];
MessageBox.Show(this.CrappyTotal);
The European Way of War: Blow your own continent up.
The American Way of War: Go over and help them.
|
|
|
|
|
What language is this? I thought it was C#, but there's no "get" or "set", and you can't declare local variables in a property.
There are three kinds of people in the world - those who can count and those who can't...
|
|
|
|
|
He probably forgot to include the get/set because he was altering the code to post it up here.
As for not declaring local variables in properties, you can.
|
|
|
|
|
Andrew Rissing wrote: He probably forgot to include the get/set because he was altering the code to post it up here.
Ah, OK - I was a bit confused (it's been a long day).
Andrew Rissing wrote: As for not declaring local variables in properties, you can.
Only inside a getter or setter, not as a "property local". I'm easily confused though...
There are three kinds of people in the world - those who can count and those who can't...
|
|
|
|
|
Yup...rightly picked up. I forgot to include the get block 
|
|
|
|
|
Perhaps he missed out the () at the end of the method name?
Between the idea
And the reality
Between the motion
And the act
Falls the Shadow
|
|
|
|
|
If the total is expected to always be computable, but it would be better to return -1 than to throw an exception if it isn't, how would you suggest coding it? Testing all four values to see if they're valid, testing to see whether any of the values is so big as to risk overflow, etc. would seem more complicated than just catching an exception. To be sure, such an approach would yield better performance in the case where an exception would be thrown and caught, but worse performance in the expected case where the total can be computed without difficulty.
With regard to the relative wisdom of returning -1 versus propagating the exception, that would depend upon what the calling program is going to do with the information. It's not hard to imagine situations where having the function return -1 will be much cleaner than having it throw an exception, and others where the reverse would be true.
BTW, one thing I would think would be useful in many situations where some callers would like exceptions handled and others would like them stifled would be for a method to accept a delegate to be called in case of trouble. Such a delegate could throw an exception if necessary, but could also handle some "trouble" situations better than would an exception.
|
|
|
|