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: This code may not quite do what it says it does.

RCoate30-Jun-11 15:28

RCoate

30-Jun-11 15:28

Thing is, this code has been in production for as least 12 months.

Re: This code may not quite do what it says it does.

Peter_in_278030-Jun-11 16:05

Peter_in_2780

30-Jun-11 16:05

So Final is obviously valid! Poke tongue | ;-P

Software rusts. Simon Stephenson, ca 1994.

Re: This code may not quite do what it says it does.

BrainiacV5-Jul-11 7:43

BrainiacV

5-Jul-11 7:43

At a former place of employment we were told "Names mean nothing." If the function name is "Print" it may not get around to doing any. And for this crapware it was true. Typically functions were not single purpose. They might do 10 different things. These functions would be called by other functions only interested in the output of two of the operations and with the hope that the other eight did not cause any problems.

A colleague and I traced a subroutine down 25 levels making calls to these routines, we totally lost what the intended result was supposed to be and never reached bottom.

Psychosis at 10
Film at 11

Re: This code may not quite do what it says it does.

User 75994145-Jul-11 5:04

User 7599414

5-Jul-11 5:04

Unless it was filling in an interface (and you have no control over the interface definition; third party, etc.), it's just code bloat. YAGNI: you ain't gonna need it. The best designs have the minimum of moving parts.

Re: This code may not quite do what it says it does.

BobJanova5-Jul-11 7:19

BobJanova

5-Jul-11 7:19

As non-virtual, I agree. I'm just suggesting what reasonable thing the original code might have been trying to do.

Re: This code may not quite do what it says it does.

timpattinson5-Jul-11 12:43

timpattinson

5-Jul-11 12:43

XKCD Deja-Vu
Image tags wouldn't work

Re: This code may not quite do what it says it does.

0bx29-Jun-11 21:23

0bx

29-Jun-11 21:23

It's funny because I know what he was thinking, I still make similar mistakes from time to time.
But when I find myself typing something like this:

if (singleApp.ApplicationId > bestMatch.ApplicationId)
{
   bestMatch = singleApp;
}
else
{
   bestMatch = singleApp;
}

It's usually a sign that it's time to slap yourself and get go outside for five minutes. Big Grin | :-D

Giraffes are not real.

Re: This code may not quite do what it says it does.

BobJanova30-Jun-11 3:26

BobJanova

30-Jun-11 3:26

Yeah I think we've all written code like that on a bad day. But most of us probably notice immediately and take a little break.

The worst thing about the original example is the way it returns a value and assigns to instance variables, I think (as well as the 'best match' logic being broken, but that's just a mistake, it's not really insidious). It could just return bestMatch and if it's null then there wasn't one, and then it wouldn't have side effects.

Re: This code may not quite do what it says it does.

Tarun.K.S30-Jun-11 3:20

Tarun.K.S

30-Jun-11 3:20

RCoate wrote:
/// <summary>
/// Does the Final validation.
/// </summary>
/// <returns></returns>
protected bool FinalValidation()
{
return true;
}

The more anger towards the past you carry in your heart, the less capable you are of loving in the present.

My Blog![^]

Re: This code may not quite do what it says it does.

chmod22225-Jul-11 1:52

chmod2222

5-Jul-11 1:52

Why don't you just do...

private bool HasApplicationAlready(int appTypeID, int registrationNumber, ref Application app)
{
	app = Application().GetApplicationListForContact(DataLocation.OnlineDataBase, Int32.Parse(Profile.RegistrationNumber)).OrderByDescending(s=>s.ApplicationId).FirstOrDefault(s=>s.Application_type_id==appTypeID);
	return app==null?false:true;
}

... or something similar?

Re: This code may not quite do what it says it does.

James H5-Jul-11 2:19

James H

5-Jul-11 2:19

Probably because it is a bitch to debug

Re: This code may not quite do what it says it does.

chmod22225-Jul-11 2:24

chmod2222

5-Jul-11 2:24

Not really.. You get null, or the result you wanted, nothing to debug.

Re: This code may not quite do what it says it does.

James H5-Jul-11 3:38

James H

5-Jul-11 3:38

But if it is null rather than the result you expected and you want to dig in a bit to find where it is going wrong it is a nightmare so you end up expanding it out a bit to trace then perhaps collapsing again once it is working (or more likely leaving well alone). While I like the concept of the single line solution until you can step through the parts I can see why you would validly avoid it.

Re: This code may not quite do what it says it does.

chmod22225-Jul-11 4:09

chmod2222

5-Jul-11 4:09

Well, when I write code, I write it so it doesn't have to be debugged(almost Smile | :)

. Only thing that can go wrong here, would be that the database connection is down, or some sore of foreign key exception. It would result in an exception(with a nice description), but that's a whole different story.

My point is, that I would always rather write(and debug) 2 lines of code, than 22, no mather how complicated they may be...but that's just me...

Re: This code may not quite do what it says it does.

austin hamman5-Jul-11 4:27

austin hamman

5-Jul-11 4:27

also im not sure the lambda notation existed(in c#) when this code was made. i may be wrong.

Re: This code may not quite do what it says it does.

chmod22225-Jul-11 4:31

chmod2222

5-Jul-11 4:31

The "not so" good old days... Smile | :)

Re: This code may not quite do what it says it does.

User 48350475-Jul-11 3:39

User 4835047

5-Jul-11 3:39

Now all of your applications are belong to us!

modified 20-Oct-19 21:02pm.

Re: This code may not quite do what it says it does.

Mark AJA5-Jul-11 9:01

Mark AJA

5-Jul-11 9:01

You need to replace the second bit of code with this:

protected bool FinalValidation()
{
  var TimeStamp = new Date();
  if(TimeStamp.getDate()%2==1){
    return false;
  } else {
    return true;
}

Re: This code may not quite do what it says it does.

Gates VP5-Jul-11 9:17

Gates VP

5-Jul-11 9:17

So that second gem is not totally useless if it's trying to fill out an Interface of some type. In PHP I've done crazy stuff like this b/c PHP only supports functions in Interfaces so you end with "constants" as "functions".

As to your first case, this has a full boat of failures. That whole block of if... elseif... else -> do the same thing is actually pretty comical. The fact that it's in a for loop and "finds something" but doesn't break is really just odd.

But the kicker to me is that he has sets the app variable and then has a return value of true. Which basically says "hey check the app variable you passed me, I hammered it and returned the app I think I found"

Thank goodness it's private and hopefully caused minimal destruction Smile | :)

Re: This code may not quite do what it says it does.

KP Lee6-Jul-11 6:41

KP Lee

6-Jul-11 6:41

This code reads like someone who codes placeholders and doesn't add comments about why he put in a placeholder. It also reads like code written by certain co-workers who can't code, comes running to you for help, mashes your suggestions into unrecognizable pulp and then presents their "work". Leaving you wondering what they were thinking.

I'll leave placeholders if I get requirements that are a confusing mishmash of illogical thinking. But I'll add comments about why my code is illogical.

Re: This code may not quite do what it says it does.

Member 80050308-Jul-11 7:10

Member 8005030

8-Jul-11 7:10

this made my day

The Engineer (wears a cowboy hat)

RobCroll22-Jun-11 20:36

RobCroll

22-Jun-11 20:36

Bumped into an interesting “Engineered” approach the other day. Went for a meeting to discuss 2 applications that share many common features. So I was expecting 2 solutions, each sharing a number of projects.

How wrong I was. It turned out to be 2 applications in the one project! I didn't react surprised because I've seen the approach before. One project liberally sprinkled with pre-processor directives.

#if APP_ABC
	//Do stuff for ABC application
#else
	//Do stuff for XYZ application
#endif

Fortunately the developer reassured me he was an Engineer a few times so that made me feel a lot of better. D'Oh! | :doh:

"You get that on the big jobs."

Re: The Engineer (wears a cowboy hat)

JV999923-Jun-11 21:54

JV9999

23-Jun-11 21:54

What no #if DEBUG vs #if RELEASE bugs in that code? D'Oh! | :doh:

Damn, he's good Poke tongue | ;-P

You should have asked him if he ever swapped dll's between the applications. That would be great fun Laugh | :laugh:

Re: The Engineer (wears a cowboy hat)

RobCroll29-Jun-11 5:16

RobCroll

29-Jun-11 5:16

Interesting approach that dll idea. I think we'll work on that Laugh | :laugh:

"You get that on the big jobs."

Re: The Engineer (wears a cowboy hat)

Sander Rossel24-Jun-11 8:04

Sander Rossel

24-Jun-11 8:04

My company actually uses such an approach for a Form. The two Forms just happen to look the same.
Every Method looks a bit like this:

If m_screenMode = ScreenMode.ScreenOne Then
   ' Do stuff.
Else
   ' Do other stuff.
End If

Fixing one mode almost always messes up the other one.
Adding Controls to the Form messes up the other one unless you do not forget to hide it etc...
It even got to a point where I had to change the name of the Form at runtime because of some other Classes that need a Formname as parameter...
Really horrible! Dead | X|

It's an OO world.

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