Jump to content

  • Log In with Google      Sign In   
  • Create Account


Dumbest coding mistakes ever


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
54 replies to this topic

#1 speciesUnknown   Members   -  Reputation: 527

Posted 06 February 2012 - 07:19 PM

languages with C like syntax allow you full control of layout etc, but this can cause bugs, like this one that kept me busy for 2 days. I had been refactoring my resource loading system and removed something from a render list class into something else, then gone for a cup of tea, leaving behind this code:

  public void AddItem(Matrix position, IRenderable3D i)
  {
   // if we have run out of space, reallocate with 2x the amount of space
   if (CurrentIndex >= capacity - 1)
    Grow(2);
   if(! i.GetMaterial().Linked)
    this.
   renderables[CurrentIndex].absolute_position = position;
   renderables[CurrentIndex].item = i;
   CurrentIndex++;
  }

Spot the error.

Another one which wasted me a whole hour of a 48 hour competition, and can be blamed on me being too keen to accept suggestions from VS2k8 intellisense: (paraphrased)
  void GameObject::SetSize(float w, float h)
  {
   this->width = w;
   this->health = h;
  }

The challenge - post your favorite dumbass coding mistakes that wasted time and/or money to find.
Don't thank me, thank the moon's gravitation pull! Post in My Journal and help me to not procrastinate!

Sponsor:

#2 Mike.Popoloski   Crossbones+   -  Reputation: 2890

Posted 06 February 2012 - 07:36 PM


void *operator new(std::size_t size, stackalloc_t stackalloc) {

	void *p = _malloca(size);

	if (!p)

		throw std::bad_alloc();

	return p;

}

Mike Popoloski | Journal | SlimDX

#3 speciesUnknown   Members   -  Reputation: 527

Posted 06 February 2012 - 08:00 PM

void *operator new(std::size_t size, stackalloc_t stackalloc) {
	void *p = _malloca(size);
	if (!p)
		throw std::bad_alloc();
	return p;
}


Is this your code? I can see that you'r e not using stackalloc, what side effect does this have? malloca still allocs on the stack right?

using /W4 usually saves me from stuff like this, although ive never had the need to override new
Don't thank me, thank the moon's gravitation pull! Post in My Journal and help me to not procrastinate!

#4 Sik_the_hedgehog   Crossbones+   -  Reputation: 1605

Posted 06 February 2012 - 10:00 PM

When writing OpenGL shaders. And I did this twice in a row. Go me:
void (main) {
   ...
}

Also not mine, but something that was mentioned on a forum a couple of years ago (ironically said forum just shut down an hour ago or so Posted Image Not kidding). Some old J2ME phones had a limit on how long functions could be, so sometimes functions had to be split. Somebody once was told to divide such a function in two. Well, guess what he did...
public void Paint() / 2
{
   ...
}

Don't pay much attention to "the hedgehog" in my nick, it's just because "Sik" was already taken =/ By the way, Sik is pronounced like seek, not like sick.

#5 speciesUnknown   Members   -  Reputation: 527

Posted 06 February 2012 - 11:48 PM

I think you missed the point -

When writing OpenGL shaders. And I did this twice in a row. Go me:

void (main) {
   ...
}

Also not mine, but something that was mentioned on a forum a couple of years ago (ironically said forum just shut down an hour ago or so Posted Image Not kidding). Some old J2ME phones had a limit on how long functions could be, so sometimes functions had to be split. Somebody once was told to divide such a function in two. Well, guess what he did...
public void Paint() / 2
{
   ...
}


I think you missed the point - both my mistakes and mikep's mistake compiled just fine but caused problems at runtime.

I cant see the code, but this happened to me a few hours ago:
Posted Image

How fucked up is that? im guessing some kind of stack corruption or something (the nuke should take 60 seconds)
Don't thank me, thank the moon's gravitation pull! Post in My Journal and help me to not procrastinate!

#6 MaulingMonkey   Members   -  Reputation: 1556

Posted 07 February 2012 - 01:33 AM


void *operator new(std::size_t size, stackalloc_t stackalloc) {
	void *p = _malloca(size);
	if (!p)
		throw std::bad_alloc();
	return p;
}


Is this your code? I can see that you'r e not using stackalloc, what side effect does this have? malloca still allocs on the stack right?

using /W4 usually saves me from stuff like this, although ive never had the need to override new


_malloca does indeed allocate on the stack. Which means the memory allocated is effectively "freed" the moment this operator returns, and will be reused by whatever the parent function calls, or even the parent function itself.

I don't remember exactly where I did this, but I did manage to pull this one off once:
int& derp = derp;

And here's a more recent moment of weakness:
if ( a != b ) a < b;

And I've tried to track down performance problems multiple times in the past year with my debugger attached... with conditional breakpoints inside inner loops...

#7 Eelco   Members   -  Reputation: 301

Posted 07 February 2012 - 02:11 AM

languages with C like syntax allow you full control of layout etc, but this can cause bugs, like this one that kept me busy for 2 days. I had been refactoring my resource loading system and removed something from a render list class into something else, then gone for a cup of tea, leaving behind this code:

  public void AddItem(Matrix position, IRenderable3D i)
  {
   // if we have run out of space, reallocate with 2x the amount of space
   if (CurrentIndex >= capacity - 1)
	Grow(2);
   if(! i.GetMaterial().Linked)
	this.
   renderables[CurrentIndex].absolute_position = position;
   renderables[CurrentIndex].item = i;
   CurrentIndex++;
  }

Spot the error.

This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

#8 way2lazy2care   Members   -  Reputation: 782

Posted 07 February 2012 - 08:03 AM

This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

Fair point, but for most of the stuff I've worked on coding standards had pretty strict rules on visually formatting the code. Stuff like this wouldn't pass code review for me:
if (CurrentIndex >= capacity - 1)
        Grow(2);
   if(! i.GetMaterial().Linked)
        this.



It would be sweet if you could make VS throw warnings for code that doesn't fit standards. You might be able to do that already, but I don't know how to do that :/

#9 speciesUnknown   Members   -  Reputation: 527

Posted 07 February 2012 - 09:32 AM


This is the classic argument for significant whitespace ala Python/Haskell. Why leave something as fundamental as the structure of your code to convention? Sure, brackets parse unambigiously enough, but then your visual cortex still has to pick up on them; it usually just relies on the indentation.

Fair point, but for most of the stuff I've worked on coding standards had pretty strict rules on visually formatting the code. Stuff like this wouldn't pass code review for me:
if (CurrentIndex >= capacity - 1)
		Grow(2);
   if(! i.GetMaterial().Linked)
		this.



It would be sweet if you could make VS throw warnings for code that doesn't fit standards. You might be able to do that already, but I don't know how to do that :/


This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.
Don't thank me, thank the moon's gravitation pull! Post in My Journal and help me to not procrastinate!

#10 way2lazy2care   Members   -  Reputation: 782

Posted 07 February 2012 - 10:32 AM

This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.

I think the standard we're working with now allows single line if statements like if(a<b) a=b, but all other control structures require blocks.

Personally, I always use blocks, so that's why I'm not positive about the above in our coding standard. I only ever don't use them with the ternary operator, and I only use the ternary operator when I'm conditionally initializing stuff because it's harder to visually parse and some people have no idea what it is.

#11 rip-off   Moderators   -  Reputation: 8113

Posted 07 February 2012 - 11:16 AM

I don't remember exactly where I did this, but I did manage to pull this one off once:
int& derp = derp;

It annoys me that this is even allowed in the language.

These are my "dumbest" moments, in C++ at least:
struct Foo
{
    Foo(float u, float v) : u(u), v(u)
    {
    }

    float u;
    float v;
}; 

A related version:

struct Bar
{ 
    Bar(float someVariableX, float someVairableY)
    :
        someVariableX(someVariableX),
        someVariableY(someVariableY) 
    {
    } 

    float someVariableX;
    float someVariableY;
}; 

lotsAndLotsAndLotsOfCode();
for(int i = 0 ; i < N ; +i) {
    lotsAndLotsAndLotsOfCode();
    frobnicate(i);
    lotsAndLotsAndLotsOfCode();
}
lotsAndLotsAndLotsOfCode();
The common problem was dismissing the problematic code as being too trivial to contain a bug, and spending lots of time debugging the calling/called code looking for errors. One gets too used to reading what one expects to be the case, rather than reading what is actually there.

#12 Cornstalks   Crossbones+   -  Reputation: 6974

Posted 07 February 2012 - 11:29 AM

I do this every once in a while. Luckily I've gotten pretty quick at catching it in the debugger, but that's only because I've gotten practice:
for (int i = someStartValue; i > 0; ++i)
{
    // code
}

I know I make plenty of others, but I'll have to try and remember them...
[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

#13 AltarofScience   Members   -  Reputation: 933

Posted 07 February 2012 - 11:30 AM

I guess I am just a bizarre abberation. I write all my code with every line on the left edge
thing=0;
otherthing=10;
while(thing<otherthing) {
stuff;
stuff;
thing++;
}
more;
code;
all;
on;
the;
left;

spacing things out generally confuses the hell out of me too.

#14 Cornstalks   Crossbones+   -  Reputation: 6974

Posted 07 February 2012 - 11:33 AM

I guess I am just a bizarre abberation. I write all my code with every line on the left edge

thing=0;
otherthing=10;
while(thing<otherthing) {
stuff;
stuff;
thing++;
}
more;
code;
all;
on;
the;
left;

spacing things out generally confuses the hell out of me too.

Yup, that's pretty weird. But there's no mistakes in teh codes....
[ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

#15 Prune   Members   -  Reputation: 217

Posted 07 February 2012 - 12:32 PM

I was writing a Windows service and needed to change the privileges of the process token.

SetPrivileges("SE_RESTORE_NAME\0SE_BACKUP_NAME\0SE_TCB_NAME\0\0");

The function would split up the string and LookupPrivilegeValue() for each privilege, building up the structure to pass to AdjustTokenPrivileges().
This worked in the previous version of Windows I was using it (I don't remember if it was XP or Vista), but in Windows 7 it didn't; LookupPrivilegeValue() was failing.

Couldn't figure it out until I looked at WinNT.h:
#define SE_RESTORE_NAME TEXT("SeRestorePrivilege")

The fix, obviously, is

SetPrivileges(SE_RESTORE_NAME "\0" SE_BACKUP_NAME "\0" SE_TCB_NAME "\0\0");

What I still don't fully understand is why it worked in older Windows 7.
"But who prays for Satan? Who, in eighteen centuries, has had the common humanity to pray for the one sinner that needed it most?" --Mark Twain

~~~~~~~~~~~~~~~Looking for a high-performance, easy to use, and lightweight math library? http://www.cmldev.net/ (note: I'm not associated with that project; just a user)

#16 CornyKorn   Members   -  Reputation: 476

Posted 07 February 2012 - 01:10 PM


while( someCondition );

{

}



#17 jwezorek   Crossbones+   -  Reputation: 1794

Posted 07 February 2012 - 01:38 PM

I often flip the direction of boolean tests. Like it should be
if (foo) ...
and I'll write
if (!foo) ...
These can be hard things to catch, because you can stare at the bad comparison but your brain just kind of assumes that the call knows what it is doing. :)

#18 Eelco   Members   -  Reputation: 301

Posted 07 February 2012 - 02:39 PM


This is why I usually have a policy of not using control structures without a block, but sometimes I relapse. It would be good to have a plugin or something that could detect this.

I think the standard we're working with now allows single line if statements like if(a<b) a=b, but all other control structures require blocks.

Personally, I always use blocks, so that's why I'm not positive about the above in our coding standard. I only ever don't use them with the ternary operator, and I only use the ternary operator when I'm conditionally initializing stuff because it's harder to visually parse and some people have no idea what it is.


Having your own standards is a partial solution; but it breaks down when you have to use some random persons code, and you are back to wishing the language enforced a standard.

#19 KanonBaum   Members   -  Reputation: 277

Posted 07 February 2012 - 02:46 PM

Trying to abuse special constructors instead of delegating the work to functions .

class Map2D
{
	Map2D() { makeGenericMap(); }
	Map2D( int width, int height, int cell_size, int layers )
	{
		 makeMap(width, height, cell_size, layers);
	}
   ...
}
...
class SpecificMap : public Map2D
{
	 SpecificMap() : Map2D()
	 {
		  MapParser parser("filename");
		
		  if( parser->isGood() )
		  {
			  Map2D::Map2D(parser->getWidth(), parser->getHeight(), ... ); // Nothing would ever happen

			  ....

              // More stuff

          }
	 }
}


I thought it'd be the same as calling any other super-function at the time. Don't know what in the world I was thinking. Spent a good bit of time tracing it down because "I thought it'd just work like that."
I'm that imaginary number in the parabola of life.

#20 alnite   Crossbones+   -  Reputation: 2069

Posted 07 February 2012 - 05:38 PM

I often flip the direction of boolean tests. Like it should be

if (foo) ...
and I'll write
if (!foo) ...
These can be hard things to catch, because you can stare at the bad comparison but your brain just kind of assumes that the call knows what it is doing. Posted Image


The worst part is when the boolean variable name adds to the logic.
if (notRunning) {
   notRunning = isRunning();
}





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS