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.

Remember - this is supposed to be C#...

6o'clock5-Mar-07 21:52

6o'clock

5-Mar-07 21:52

///

/// Fuegt solange Blanks an den String an, bis die
/// Laenge length erreicht wird.
///

/// <param name="inputString" />
/// <param name="length" />
/// <returns>
public static string AddBlanks(string inputString, int length)
{
string outString = inputString;

while (outString.Length < length)
{
outString = outString + " ";
}

return outString;
}

Not only a memory issue (using string instead of stringbuilder), but why have the gods^W BCL group created the string.PadRight(lenth, ' ') method?

What do you think? Laugh | :laugh:

Cheers,

Juergen

Re: Remember - this is supposed to be C#...

jhwurmbach8-Mar-07 4:42

jhwurmbach

8-Mar-07 4:42

6o`clock wrote:
Not only a memory issue (using string instead of stringbuilder), but why have the gods^W BCL group created the string.PadRight(lenth, ' ') method?

What do you think?

I would think someone is learning C#.

I can see me making mistakes of exactly the same type.
Ok, I know about stringbuilder, but there are whole top-level namespaces I never used. I certainly do not know their functions by heart.

Failure is not an option - it's built right in.

Re: Remember - this is supposed to be C#...

Erik Burger9-Mar-07 1:10

Erik Burger

9-Mar-07 1:10

My colleague and I had an interesting discussion about this. According to us, there are two types of programmers out there. The first type thinks 'This is such a common thing to do, someone MUST have included it' and therefor ends up using the PadString method. The second type just churns away without seconds thoughts and ends up with the originally posted code.

The funny thing is that 9 out of 10 times people tend to refactor these snippits into a utilities class, calling the method 'PadString' Cool | :cool:

We always use "i" as a loop counter...

Hal Angseesing5-Mar-07 21:29

Hal Angseesing

5-Mar-07 21:29

...so lets make it global.
This was the answer of a "Technical Consultant" that I worked with many years ago. All of his loops were for i = 1 to 10 next i. This was VB 3 - so some time ago!

To save himself the effort of dim i as integer at the beginning of every function he wanted to use "i" he just made it a global. I can't blame him too much - he was a COBOL programmer who had made the leap to VB.

It made for some surprisingly subtle bugs! Needless to say that I removed it after having to try quite hard to show him the benefits (I was a junior C++ programmer at the time).

Re: We always use "i" as a loop counter...

BSelznick6-Mar-07 15:19

BSelznick

6-Mar-07 15:19

We must have worked with the same guy! I once worked on a VB program with a couple of hundred global variables. When i complained to the original developer he said, "What's wrong with that?"

Funny, this guy was an author of multiple VB books.

Bill Selznick
Bits 2 Systems LLC

bselznick@bits2systems.com

Re: We always use "i" as a loop counter...

Ted Ferenc6-Mar-07 21:32

Ted Ferenc

6-Mar-07 21:32

many years ago I saw the same years ago in a FORTRAN prog, as you say some of the bugs this introduced were interesting. Perhaps this guy used to work in England?

"Normal is getting dressed in clothes that you buy for work and driving through traffic in a car that you are still paying for - in order to get to the job you need to pay for the clothes and the car, and the house you leave vacant all day so you can afford to live in it." - Ellen Goodman

Re: We always use "i" as a loop counter...

Duncan Edwards Jones7-Mar-07 1:10

Duncan Edwards Jones

7-Mar-07 1:10

Had this man not heard of DefInt? Laugh | :laugh:

(Worst keyword ever - except perhaps As Any!)

'--8<------------------------
Ex Datis:
Duncan Jones
Merrion Computing Ltd

Re: We always use "i" as a loop counter...

John R. Shaw9-Mar-07 11:30

John R. Shaw

9-Mar-07 11:30

Hal Angseesing wrote:
...so lets make it global.

I am cringing at the very thought of that! Laugh | :laugh:

The largest application (written in C) I ever worked on had more global variables than I bothered to count. I was not allowed to create the original but I was the one who had to fix it for release to the public. When I examined the original code, for the first time, I found a dozen ways to crash the system within 15 minutes of opening the sources files, what a mess. OMG | :OMG:

Do not worry! The finished product worked just as required and would no longer crash the system if looked at it wrong. Wink | ;)

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

Infinite for loops

Jesse Krebs5-Mar-07 21:00

Jesse Krebs

5-Mar-07 21:00

Hello, love the show, long time listener, first time caller.

One of the company's c++ coding guidelines is that goto's are forbidden. For some developers who use goto statements as error handling this could present a problem. For others, ...

for ( ;; ) {
    // Some code
    for ( ;; ) {
        bool someError = func();
        if (someError) {
           break;
        }
        // ...
        if (anotherError) {
           break;
        }
        // ...
        break;
    }
    // ...
    for ( ;; ) {
       // A few more hundred lines of code with more for( ;; ) loops.
       break;
    }
    // ...
    break;
}

After stumbling upon this beauty I searched and found over 200 infinite loops used as an error handling technique. Not one of these loops ever actually looped.

Re: Infinite for loops

Mike Dimmick6-Mar-07 3:48

Mike Dimmick

6-Mar-07 3:48

Yep, seen it. I hate this irrational ban on goto. It's out of date - it dates from a time when decent control structures weren't common. A judicious, careful, occasional use of a goto is fine. Subverting a control structure to give it a different meaning is definitely not.

For more, see Gotos Considered Harmful and Other Programmers' Taboos[^] [PDF, 17k].

Stability. What an interesting concept. -- Chris Maunder

Re: Infinite for loops

RodgerB6-Mar-07 9:42

RodgerB

6-Mar-07 9:42

Goto's are really really bad and should be super rarely used. They are not needed for error handling. The only good use I've seen for goto's is machine generated code with tools like YACC and LEX.

Generally speaking, break, continue, and multiple returns can cause problems too. Readability and maintainability are very important.

Imagine if you will a "come from" statement. You put a label X somewhere then in a totally different part of the method you place a "come from" label X. Whenever control reaches the label you goto the "come from". Imagine how hard it would be to read that code.

What's wrong with using exceptions for error handling?

Re: Infinite for loops

jhwurmbach8-Mar-07 4:56

jhwurmbach

8-Mar-07 4:56

RodgerB wrote:
Goto's are really really bad and should be super rarely used. They are not needed for error handling.

This

  if (spin_trylock(&tty_lock.lock))
    goto got_lock;
  if (tsk == tty_lock.lock_owner) {
    WARN_ON(!tty_lock.lock_count);
    tty_lock.lock_count++;
    return flags;
  }
  spin_lock(&tty_lock.lock);
got_lock:
  WARN_ON(tty_lock.lock_owner);

seems pretty well justified for me.
You could do this with nested ifs, but you would neither get more clarity nor faster code. Only code indenting out at your right screen margin.

Failure is not an option - it's built right in.

Re: Infinite for loops

RodgerB8-Mar-07 7:17

RodgerB

8-Mar-07 7:17

I guess we could argue this back and forth. It comes down to avoiding mistakes and doing things as a matter of policy.

E.g.

I never do this:

int* y, z;

I always do this:

int* y;
int z;

Why you ask. Too easy to read it wrong at a glance, too easy to miss something. Some logic as to why I never use goto's and why I don't put returns anywhere but at the very bottom or very top of a function.

I believe this to be more than just a matter of style.

Re: Infinite for loops

John R. Shaw9-Mar-07 12:29

John R. Shaw

9-Mar-07 12:29

That is an excellent paper! I rarely use goto, but sometimes it is the best solution to a problem. (Example: escaping from multiple nested loops.)

I once read someone’s general rule that you always go forward, but I found one exception years ago:

NEXT_BYTE:
if( x>xmin ) {
    Byte = *(--vptr);
    /* Skip whole bytes */
    if( Byte == 0xFF ) {
        x -= 8;
        goto NEXT_BYTE;
    }
}

Sure the above code could be written using a loop and originally was, but this increased the speed (graphics code) enough to justify its use at the time.

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

Database rats nest

Steve Fillingham5-Mar-07 19:26

Steve Fillingham

5-Mar-07 19:26

I once came across an Access database (already WTF) that used columns instead of rows for a Calendar like application ! The Calendar table had (from memory) 1 row for each day with 50+ columns for each half hour block from 8:00am. Now you can imagine the horror it was to try to maintain such an application. The reports that ran of this dog were nightmarish, even to SELECT a single day that had 'something' happening on it looked something like

SELECT * FROM Calendar WHERE Time800 <> '' OR Time830 <> '' OR Time900 <> '' .. OR Time1600 <> ''

I can imagine what the coder would have said when asked to make the days start a little earlier than 8:00, "Oh, sure, that'll take at least another 4 months to re-schema the database and touch up all the affected queries".

And when the original developers were sacked for taking way took long and we took over the project we were questioned why we were completely rewriting it from scratch. The above Calendar table was just one example in this diseased rats nest, there were plenty of others like using hidden form controls as global variables. You would open up the main form and had absolutely no hope of actually telling what it would look like when run since it was completely covered in these demons from hell. I just hate with a passion code that does not read simple, I hate it when I cannot confidently make a simple change to some code without it screwing up something completely unrelated because the variable I modded was global and had different meanings depending on where it was called from.

Anyway that's my rant Smile | :)

Code should be simple. No excuses.

Re: Database rats nest

Dirk Gutsche5-Mar-07 21:59

Dirk Gutsche

5-Mar-07 21:59

Steve Fillingham wrote:
1 row for each day with 50+ columns for each half hour block from 8:00am

More than 50 half hours after 8am? Which planet was that calendar designed for - mercury or venus?!? Wink | ;)

Re: Database rats nest

Bradml6-Mar-07 0:53

Bradml

6-Mar-07 0:53

I wager the developers put in extra hours for leap years.

Brad
Australian

- Captain See Sharp on "Religion"

any half intelligent person can come to the conclusion that pink unicorns do not exist.

Re: Database rats nest

Steve Fillingham6-Mar-07 19:32

Steve Fillingham

6-Mar-07 19:32

Good pickup Smile | :)

I dont know about the Calender but the dev was definitely not of this Earth Smile | :)

if something then... nothing

Fernando A. Gomez F.5-Mar-07 13:27

Fernando A. Gomez F.

5-Mar-07 13:27

This jewel is one of my favourites. I found it in a customer's code (they asked us to enhance the application):

BOOL cIBatchDownload::OnInitDialog()
{
  if (m_bCreated)
  {
  }

  ...

  return TRUE;
}

I eager to look deeper into this code to find more post for this newly created forum. Badger | [badger,badger,badger,badger...]

A polar bear is a bear whose coordinates has been changed in terms of sine and cosine.
Personal Site

Re: if something then... nothing

John R. Shaw9-Mar-07 12:38

John R. Shaw

9-Mar-07 12:38

Hmmm!

I have done that when outlining the code before filling the in-betweens. The real question is: what was supposed to be inserted there?

INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra

Object identifiers [modified]

_oti5-Mar-07 13:12

_oti

5-Mar-07 13:12

Once upon a time, I was leading a team that was doing the server side of an internet banking application. Because this sort of app was in its infancy, there was little confidence, and a lot of paranoia. IIRC, the flow was that a connection would come into an external server, a session would be created and packed and sent across a serial cable to the machine that would handle authentication. This packet would be decrypted, verified and sent back, then it would move on to another machine across another serial cable for the transaction. We had essentially the same process running on each machine, but operating a bit differently depending on the role.

ANYWAY ... we had a problem in that sometimes, a session would drop out, and the service would crash. We logged, we debugged, we stepped through, and couldn't find it, until one fine day, we hit a breakpoint, and the developer said "Huh. That's funny. That should be a session object, not a data chunk."

The penny dropped. "Ah," I said. "That session ID that you're passing back and forth between machines ... is that an index into the session array, or what?"

"No," came the bemused reply. "I thought it would be quicker to just use the address of the object".

Apparently, the original object had been deleted because the session had been dropped, and the runtime had reused the space for something else. So when the session ID was cast from its session ID form (int) to its pointer form, it was pointing at something else. Don't get me wrong ... I love pointers. I just had no idea that their abuse would be extended to neighbouring machines.

-- modified at 23:41 Monday 5th March, 2007

--
All things considered, you can't really consider all things ...

duplicate variable names seperated by scope

Jason De Arte5-Mar-07 13:08

Jason De Arte

5-Mar-07 13:08

I was once maintaining some odd looking code that had the (no joke) the following pattern:

<br />
int variable;<br />
<br />
class CSomething {<br />
public:<br />
 int variable;<br />
<br />
  void Func(int variable) {<br />
  // do something<br />
  {<br />
    int variable;<br />
    // do something where the bug I was trouble shooting was located<br />
  }<br />
 };<br />
};<br />

Count them: FOUR declarations of the same variable name (and type) in the same file!
1) In the global scope
2) As a member var
3) as a function parameter
4) declared IN SIDE the function that has the identical named param

Yes, it was legal - but it didn't make it right.

[ Jason De Arte | Toy Maker | 1001010.com ]

Re: duplicate variable names seperated by scope

bluesboy847-Mar-07 1:06

bluesboy84

7-Mar-07 1:06

Makes me remind of the last programming exams i took... They make you go insane with a snippet 10 times difficult than the above, then they ask you "WTF does var on line x refer to, which is its value?"
Someone just never learns Wink | ;)

Unnecessary try / catch block

David Rush5-Mar-07 10:17

David Rush

5-Mar-07 10:17

Whilre reviewing the code from some contract developers, I have found a common construct:

<br />
public void SomeMethod()<br />
{<br />
    try<br />
    {<br />
        // some code here...<br />
    }<br />
    catch (Exception exception)<br />
    {<br />
        throw exception; // Just re-throw<br />
    }<br />
}<br />

Why just catch and re-throw an exception? This just adds to the complexity of the execution without doing anything in return.

Re: Unnecessary try / catch block

PIEBALDconsult5-Mar-07 12:52

PIEBALDconsult

5-Mar-07 12:52

I would wrap that in an # if DEBUG; too many times when debugging I can't figure out where an exception happened, especially when there's a finally involved.

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