|
dakovinc wrote: the user has no idea what just happened
Do they ever have any idea?
|
|
|
|
|
Sadly, no. (or almost never)
This is especially true for the users of the program where this code came from.
|
|
|
|
|
I'd be looking for other related error messages:
MessageBox.Show("Danger Will Robinson");
MessageBox.Show("Never fear, Smith is here!");
|
|
|
|
|
Found this in code I wrote a long time ago. Not the stupidest code ever perhaps, but it's just, well, see for yourself.
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)
{
SpreadCompressed(width, width > 21, height > 10, xpos, ypos);
}
else
{
Spread(width, 8, 7, xpos, ypos);
}
}
else
{
Spread(width, 9, 9, xpos, ypos);
}
}
else
{
Spread(width, 9, 10, xpos, ypos);
}
}
else
{
Spread(width, 11, 12, xpos, ypos);
}
}
else
{
Spread(width, 13, 14, xpos, ypos);
}
}
else
{
Spread(width, 15, 16, xpos, ypos);
}
|
|
|
|
|
I used to have a flash drive with code projects on it from my first semester in college. It was a unique set of code
Flash drive died though, and I never had a backup.
If it moves, compile it
|
|
|
|
|
This is a good example of even when there are a lot of nesting levels, the code can be formatted so that it is readable and understandable.
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
posting about Crystal Reports here is like discussing gay marriage on a catholic church’s website.[Nishant Sivakumar]
|
|
|
|
|
Yes, and I can't remember why I wrote it this way. It should never even have happened in the first place.
|
|
|
|
|
So what's the alternative?
|
|
|
|
|
Well I could have done this:
if (width <= 11 && height <= 4)
{
size = 29;
Spread(width, 15, 16, xpos, ypos);
}
else if (width <= 13 && height <= 5)
{
size = 30;
Spread(width, 13, 14, xpos, ypos);
}
else if (width <= 15 && height <= 6)
{
size = 31;
Spread(width, 11, 12, xpos, ypos);
}
else if (width <= 18 && height <= 7)
{
size = 32;
Spread(width, 9, 10, xpos, ypos);
}
else if (width <= 19 && height <= 8)
{
size = 33;
Spread(width, 9, 9, xpos, ypos);
}
else if (width <= 21 && height <= 10)
{
size = 34;
Spread(width, 8, 7, xpos, ypos);
}
else
{
size = 34;
SpreadCompressed(width, width > 21, height > 10, xpos, ypos);
}
Not great either, but still better I would say. At least the width and height requirements are actually grouped together with what should happen if they're met, instead of in a weird inverted way moved as far apart as possible.
|
|
|
|
|
Perhaps a map with a key class for width and height, and a value class for the passed parameters. Use std::map::upper_bound() to find the element you're looking for....
then it'll be dynamic with no hard coding...
|
|
|
|
|
It's over 9,000!!!!!!
I assume you have been waiting for that.
|
|
|
|
|
harold aptroot wrote: code I wrote a long time ago You dare admit that here?
Although I must admit I've written something like five nested if's just today...
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
How about something along the lines of:
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); break;
}
|
|
|
|
|
There is no size 35 though, so I'm afraid case 34 would have to be a bit ugly..
|
|
|
|
|
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--;
break;
|
|
|
|
|
Is size used elsewhere?
I would have probably calculated a widthSize and a heightSize separately and then
size = max(widthSize, heightSize)
This approach basically replaces six || operators with one compare in the max() function.
You could have used an array, switch statement, collection or even if statements for calculating/looking up widthSize and heightSize.
switch would probably be most readable (and fastest for these datapoints).
|
|
|
|
|
I don't know what to say so :
pfft....
Rtn = ""
Rtn = Rtn & " Some text " & vbCrLf
... repeat that many times ...
Content = Rtn
MainInfo = WrapContent(Content)
Function WrapContent(connection,content)
Rtn = ""
Rtn = Rtn & ... logic for gathering more information (all in h4 tags) with vbCrLf ..
if Content > "" then
Rtn = Rtn & "" & vbCrLf
Rtn = Rtn & Content
xRtn = Rtn
Rtn = HtmlReFormat(xRtn & vbCrLf)
end if
WrapContent = Rtn
End Function
Function HtmlReFormat(Content)
Content = Replace(Content,vbCRLF,"<br>")
Content = Replace(Content," "," ")
End Function
Variable = trim(rs("some value"))
Variable = iif(Variable,"=","value","Something",Variable)
Variable = iif(Variable,"=","other value","Something",Variable)
Variable = iif(Variable,"=","yet another value","Something",Variable)
... a few more times ...
Variable = iif(Variable,"=","","Undefined",Variable)
Function IIF(sVal1, Equal, sVal2 , IfTrue, IfFalse)
dim Rtn
Equal = trim(Equal)
select case Equal
case "="
if sVal1 = sVal2 then
Rtn = IfTrue
Else
Rtn = IfFalse
End If
case "<"
if sVal1 < sVal2 then
Rtn = IfTrue
Else
Rtn = IfFalse
End If
case ">"
if sVal1 > sVal2 then
Rtn = IfTrue
Else
Rtn = IfFalse
End If
case "<="
if sVal1 <= sVal2 then
Rtn = IfTrue
Else
Rtn = IfFalse
End If
case ">="
if sVal1 >= sVal2 then
Rtn = IfTrue
Else
Rtn = IfFalse
End If
end select
IIF = Rtn
End Function
If it moves, compile it
|
|
|
|
|
Wow...
You should stick an Option Explicit in there and wait for the guy to come complaining about a bunch of compile errors that weren't there yesterday...
And I love how he defines his own "enhanced" version of IIf...
|
|
|
|
|
Have a 5 for the sheer level of involved in this.
|
|
|
|
|
I usually don't post his code on here. There are mountains of stuff in everything he touches. My first job when I got here was to learn a couple of things he was in charge of so that SOMEONE in the company would understand what the heck is going on with them.
Everything that involves him that I have to do gives me a headache. It takes me longer to read and understand what is going on with his code than it would for me to write it again from scratch.
Funny thing is, it takes him just as long to explain his code as it would me. It's faster to read it yourself than to ask him a question about it because he has to go through the same process (re-reading, trying to figure out what the heck is going on) as I do.
Really, I can't even convey what type of coding he does.
If it moves, compile it
|
|
|
|
|
loctrice wrote: Really, I can't even convey what type of coding he does.
The term "Cowboy Coding" comes to my mind. ('cause the coder rides roughshod all over the code)
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
posting about Crystal Reports here is like discussing gay marriage on a catholic church’s website.[Nishant Sivakumar]
|
|
|
|
|
I say he's not a programmer , he's a script kiddie. He can make a computer do something given enough time, but has no place in professional code.
If it moves, compile it
|
|
|
|
|
Think I got as far as & vbCrLf before I knew it was going to be some fun.
Thanks! It certainly didn't disappoint.
|
|
|
|
|
loctrice wrote: I don't know what to say
I second that. Well... I know what to say, but there's too much and 99.999% of it is not really kid sister safe to say
"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
|
|
|
|
|
Wow, that is some of the worst garbage code I have seen and I have seen a lot of poor code.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|