The Weird and The Wonderful

The Weird and The Wonderful forum is a place to post Coding Horrors, Worst Practices, and the occasional flash of brilliance.

We all come across code that simply boggles the mind. Lazy kludges, embarrassing mistakes, horrid workarounds and developers just not quite getting it. And then somedays we come across - or write - the truly sublime.

Post your Best, your worst, and your most interesting. But please - no programming questions . This forum is purely for amusement and discussions on code snippets. All actual programming questions will be removed.

Re: Performance Genius

Steve Caine27-Dec-10 10:52

Steve Caine

27-Dec-10 10:52

Definitely. There's no excuse for

for ( int i = 0; i < data.Count; i++ )

instead of the obvious, clearly superior and faster

for ( int i = 0; i < data.Count; ++i )

Poke tongue | ;-P

Re: Performance Genius

BillW3329-Dec-10 4:23

BillW33

29-Dec-10 4:23

Yes, it is surprising how few C++ programmers know that ++i executes faster than i++ Smile | :)

Just because the code works, it doesn't mean that it is good code.

Re: Performance Genius

ghle29-Dec-10 11:17

ghle

29-Dec-10 11:17

CIDev wrote:
Yes, it is surprising how few C++ programmers know that ++i executes faster than i++ Smile

Look at the assembly and you'll see why.
Fewer instructions to execute. Thumbs Up | :thumbsup:

Gary

Re: Performance Genius

Steve Caine29-Dec-10 20:02

Steve Caine

29-Dec-10 20:02

Because ++i doesn't have to keep track of/return the previous value of i.

Re: Performance Genius

ely_bob29-Dec-10 7:06

ely_bob

29-Dec-10 7:06

The really sad thing is that most programmers don't know that i-- is the fastest way of the four... Big Grin | :-D

I'd blame it on the Brain farts.. But let's be honest, it really is more like a Methane factory between my ears some days then it is anything else...

-----

"The conversations he was having with himself were becoming ominous."-.. On the radio...

Re: Performance Genius [modified]

#realJSOP2-Jan-11 1:33

#realJSOP

2-Jan-11 1:33

Is there some subtle sarcasm going on here? I've never seen "++i" used (in a for loop) - ever.

EDIT ==========

Just to see if you guys are chain yankin' here, I tried it both ways :

1 million iterations = both completed the iteration in 0.0040002 seconds
10 million iterations = i++ took 0.0350020, and ++i took 0.0340019 seconds

".45 ACP - because shooting twice is just silly" - JSOP, 2010
-----
You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass." - Dale Earnhardt, 1997

modified on Sunday, January 2, 2011 9:42 AM

Re: Performance Genius

fjdiewornncalwe2-Jan-11 4:25

fjdiewornncalwe

2-Jan-11 4:25

Does this mean the wife is not up yet as is the case for me and that it is best to let her sleep in on Sunday morning?
(Might be a good idea considering she's quick on the draw)... Smile | :)

I wasn't, now I am, then I won't be anymore.

Re: Performance Genius

Steve Caine3-Jan-11 10:35

Steve Caine

3-Jan-11 10:35

John Simmons / outlaw programmer wrote: "Is there some subtle sarcasm going on here? I've never seen "++i" used (in a for loop) - ever."

Somewhat subtle. The increment operator is slightly more efficient in its prefix version (++i) because it just increments 'i' and returns its value. The postfix version has to do more work because it returns the *previous* version of 'i', so it has to store that value somewhere before incrementing i.

If you're just incrementing 'i', it makes more sense to use the more efficient prefix version. It's just better form, and encourages good habits. (For example, applying it to something more complex than an integer, say an iterator to an STL container class, might produce a more significant performance hit than the miniscule one your test found.)

Yet you're right, most of the instances of '++' or' --' I've seen in code, particularly in 'for' loops, is the postfix version. There's no reason for it, yet somehow that has become a near-universal meme in programming. Perhaps because the most common example beginning programmers encounter is the classic C-string copy code snippet, where you really *do* want to use the postfix version:

while (*dst++ = *src++)
;

So I was making a joke that the real coding outrage in the original poster's message was using 'i++' instead of '++i', as if I had completely missed the real outrage of sleeping the thread in each pass through the 'for' loop.

(Getting off my soapbox now.)

Re: Performance Genius

Sander Rossel27-Dec-10 19:47

Sander Rossel

27-Dec-10 19:47

Isn't there a version 2.5? Thread.Sleep(0); Poke tongue | ;-P

Re: Performance Genius

Ravi Sant27-Dec-10 22:37

Ravi Sant

27-Dec-10 22:37

another version may come up with something like
for ( int i = 0; i < data.Count; container.Add(data[i].Items,i++ )

Re: Performance Genius

yxhu5-Jan-11 13:10

yxhu

5-Jan-11 13:10

improved version of above can be like:
for ( int i = 0; i < data.Count; container.Add(data[i++].Items)

Re: Performance Genius

Adrian020-Jan-11 1:48

Adrian0

20-Jan-11 1:48

I can improve even that code
for ( int i = 0; i < data.Count; container.Add(data[++i].Items)

Re: Performance Genius

yxhu23-Jan-11 11:50

yxhu

23-Jan-11 11:50

Adrian0, unfortunately, your solution was invalid.
As ++i will be calculated(increased) before accessing the element of data collection.
container will never have data[0], plus you will receive an out of index exception at the final loop
of this for expression. Say data collection has 4 elements.
Each iteration looks like:

Loop 1, i initialised = 0:
container.Add(data[1]).Items

Loop 2, i initialised = 1:
container.Add(data[2]).Items

Loop 3, i initialised = 2:
container.Add(data[3]).Items

Loop 3, i initialised = 3:
container.Add(data[4]).Items <<< Exception here

When i++ or ++i is within an expression, ++i has the highest priority to get executed bofere the reset of
the expression. In contrast, i++ is get executed after the whole expression has been executed.

container.Add(data[i++].Items) //Get the ith element of data collection, append to container and then increase i by 1
container.Add(data[++i].Items) //Increase i by 1, then get the 1th element of data collection to append to the container.

Re: Performance Genius

Adrian023-Jan-11 12:44

Adrian0

23-Jan-11 12:44

Good work yxhu. Didn't recognize that.

But if you are that fussy watch your parenthesis

Loop 3, i initialised = 3:
container.Add(data[4]).Items <<< Exception here

Your code wouldn't compile because .Add() is usually of type void. Wink | ;)

greets
Adrian

Re: Performance Genius

yxhu23-Jan-11 13:14

yxhu

23-Jan-11 13:14

Lol... that was a typo. Thanks Adrian0. Thumbs Up | :thumbsup:

Useless function

musefan23-Dec-10 3:39

musefan

23-Dec-10 3:39

A common one in this forum is functions used that don't actually do anything. Well here is one I have just found too...

public static bool ToBoolean(bool value);

...forget where I seen this now Laugh | :laugh:

(no wonder it is seen so often, people obviously read too much documentation lol)

PS. I am assuming most no exactly where I found this Convertion function

return 5;

Re: Useless function

OriginalGriff23-Dec-10 5:46

OriginalGriff

23-Dec-10 5:46

It not really a horror, is it? It does say it does no conversion in MSDN[^]
I would say it is just there to complete the set: Convert.ToInt32 and so forth are there too.

Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.

Re: Useless function

musefan27-Dec-10 6:25

musefan

27-Dec-10 6:25

As I said. People post these 'Do Nothing' functions all the time. I was simple pointing this one out as these are obviously OK if MS says so

return 5;

Re: Useless function

RobCroll24-Dec-10 4:29

RobCroll

24-Dec-10 4:29

Actually very handy if you are passing a object type which has an underlying value of type bool. Good code should always be strongly typed but maybe when dealing with COM objects, this overloaded method could be handy.

Re: Useless function

musefan27-Dec-10 6:27

musefan

27-Dec-10 6:27

Robert Croll wrote:
Actually very handy if you are passing a object type which has an underlying value of type bool

But this specific function does not take an object. It takes a bool and returns a bool. Just wasted code in any usage

return 5;

Re: Useless function

fjdiewornncalwe27-Dec-10 5:45

fjdiewornncalwe

27-Dec-10 5:45

For those who desperately need self-validation.

I wasn't, now I am, then I won't be anymore.

Recursive methods. [modified]

Sander Rossel22-Dec-10 9:28

Sander Rossel

22-Dec-10 9:28

I love (unending) recursive methods...
I recently had one which took me some time to fix too.
I needed to get a list with all the controls in a form.

Some code calls this function with the current form as obj and an empty list of controls as list.

Private Sub GetControls(Byval obj as Control, ByRef list as List (Of Control))

For each ctl as Control in obj.Controls
   If ctl.Controls.Count > 0
      GetControls(obj, list)
      list.Add(ctl)
   Else
      list.Add(ctl)
   End If
Next ctl

End Sub

I just didn't see I kept passing on the same form over and over again Laugh | :laugh:

Even better is a recursive method I recently found in production code.
Our company uses their own controls which have a Mandatory property.
So before saving data we want to check if all controls that have Mandatory set to true have a value.
I don't remember the actual names of the methods, but it doesn't really matter anyway Wink | ;)

Public Sub Save()
' Some code here

If canSave(Me) Then
   SaveNew()
End If

' Some more code here
End Sub

Private Function canSave(ByVal obj as Control) As Boolean
Dim bool as Boolean = True

For each ctl as Control in obj.Controls
   If ctl.Controls.Count > 0
      canSave(ctl)
      If TypeOf ctl is CompanyControl Then
         Dim cctl As CompanyControl = ctl
         If cctl.Mandatory AndAlso cctl.Value = "" Then
            bool = False
         End If
      End If
   Else
      If TypeOf ctl is CompanyControl Then
         Dim cctl as CompanyControl = ctl
         If cctl.Mandatory AndAlso cctl.Value = "" Then
            bool = False
         End If
      End If
   End If
Next ctl

Return bool

End Function

The form it was used on had only 3 controls outside a container.
So I added a mandatory field on a tab, kept it empty and...
See how that would return true in most cases? Smile | :)

modified on Wednesday, December 22, 2010 3:35 PM

Re: Recursive methods.

_Erik_23-Dec-10 3:58

_Erik_

23-Dec-10 3:58

The horror here is not only that these methods do not do what they are supposed to do. The fact is that the implementation itself is really poor. Seems that the programmer got paid according to the number of lines of code he wrote.

Re: Recursive methods.

Sander Rossel23-Dec-10 5:15

Sander Rossel

23-Dec-10 5:15

Well... Actually I (the new developer) has to pay Frown | :(

Re: Recursive methods.

RobCroll24-Dec-10 4:46

RobCroll

24-Dec-10 4:46

Hopefully the programmer was so called "Senior". That's a tough call for a junior programmer.

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.