Advertisement

Dumbest coding mistakes ever

Started by February 07, 2012 01:19 AM
53 comments, last by Bacterius 12 years, 6 months ago
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!

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
Advertisement


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!
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 sad.png 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.
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 sad.png 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:
fuuuu.jpg

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!

[quote name='Mike.Popoloski' timestamp='1328578594' post='4910373']

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
[/quote]

_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;

[color=#000000]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...
Advertisement

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.

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:
[source]
if (CurrentIndex >= capacity - 1)
Grow(2);
if(! i.GetMaterial().Linked)
this.

[/source]

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 :/

[quote name='Eelco' timestamp='1328602267' post='4910440']
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:
[source]
if (CurrentIndex >= capacity - 1)
Grow(2);
if(! i.GetMaterial().Linked)
this.

[/source]

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 :/
[/quote]

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!

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.

This topic is closed to new replies.

Advertisement