What's the worst line of code you can remember writing?
56 replies, posted
I wrote a line of code a couple of days ago in C# that I actually ended up leaving a comment apologizing for
It was something along the lines of
foreach (int i in new int[] {-1,1}) {
I'm sure it's not the worst line of code I've ever written but it's pretty bad (at least I think it is, I might be wrong actually)
Have you ever found yourself writing an especially bad line of code because you couldn't think of any good alternatives?
[QUOTE=Helix Snake;52140486]
Have you ever found yourself writing an especially bad line of code because you couldn't think of any good alternatives?[/QUOTE]
Every singe day man. That's what happens when you need to fix things in production every day.
int rem = num - ((num/div)*div)
before I knew what mod was I abused integer division to do the same thing
[QUOTE=Helix Snake;52140486][...]
foreach (int i in new int[] {-1,1}) {
[...][/QUOTE]
I don't remember any right now (There's probably some awful LINQ abuse.), but I'm really tempted to make this work (somewhat) efficiently now :v:
When I made my first attempt at my C++ voxel engine, which was my real first attempt at C++, period, I didn't know about standard library containers and used this to setup my voxels:
[cpp]
Chunk::Chunk(){
chunkBlocks = new Block**[CHUNK_SIZE];
for (int i = 0; i < CHUNK_SIZE; ++i) {
chunkBlocks[i] = new Block*[CHUNK_SIZE];
for (int j = 0; j < CHUNK_SIZE; ++j) {
chunkBlocks[i][j] = new Block[CHUNK_SIZE_Z];
}
}
}[/cpp]
which is all kinds of gross and makes me wonder how that tutorial got so popular, since it wasn't written in 1999 and there's not a TON of reasons to not just use std::vector? or in this, case std::array since the size of chunks can be determined at compile-time?
also, in this same project, i had a function to build a single "cube" mesh that checked the faces of each cube to decide if it needed to mesh that face or not. then, it called a function that generated the appropriate faces based on a whole pile of booleans. anyways, instead of even understanding what lambda functions are I had 6 variations of this same chunk of code, for each face:
[cpp]
if (frontFace == false) {
index_t i0, i1, i2, i3; vertType vert0, vert1, vert2, vert3; // Using Points 0, 1, 2, 3 and Normal 0
vert0.position = vertices[0]; vert1.position = vertices[1];
vert2.position = vertices[2]; vert3.position = vertices[3];
vert0.normal = normals[0]; vert1.normal = normals[0];
vert2.normal = normals[0]; vert3.normal = normals[0];
i0 = this->mesh.addVert(vert0); i1 = this->mesh.addVert(vert1);
i2 = this->mesh.addVert(vert2); i3 = this->mesh.addVert(vert3);
this->mesh.addTriangle(i2, i1, i0); this->mesh.addTriangle(i3, i2, i0);
}
[/cpp]
also, I had the ever-classic sign of bad programming: "using namespace std;" D:
When I was 13 and fumbling around in Unity I used to write a LOT of shit. I'm pretty sure I posted something on here akin to:
[CODE]someGameObject.GetComponent<ClassThatHoldsAGameObjectReference>().gameObjectReference.transform.position = [Some random vector3 that doesn't matter][/CODE]
It didn't bother me that doing shit like this made it impossible to see all my code at once. In fact, I don't even think that thought crossed my mind :v:
[QUOTE=Tamschi;52140777]I don't remember any right now (There's probably some awful LINQ abuse.), but I'm really tempted to make this work (somewhat) efficiently now :v:[/QUOTE]
[editline]edit[/editline] [URL="https://gist.github.com/Tamschi/2f978e833ee102e36118a9cbf8c87d20"]:snip: Moved to Gists since it's off-topic.[/URL]
This might qualify if it was a single line :science101:
Pretty much my entire current project
[CODE]
$userDetails = json_encode($user);
$userDetails = json_decode($userDetails, true); // I know this is bad... Dont blame me :([/CODE]
All due to some random things i needed being unruly. PHP objects are both good and bad.
[QUOTE=paindoc;52140782]When I made my first attempt at my C++ voxel engine, which was my real first attempt at C++, period, I didn't know about standard library containers and used this to setup my voxels:
Chunk::Chunk(){ chunkBlocks = new Block**[CHUNK_SIZE]; for (int i = 0; i < CHUNK_SIZE; ++i) { chunkBlocks[i] = new Block*[CHUNK_SIZE]; for (int j = 0; j < CHUNK_SIZE; ++j) { chunkBlocks[i][j] = new Block[CHUNK_SIZE_Z]; } }}
which is all kinds of gross and makes me wonder how that tutorial got so popular, since it wasn't written in 1999 and there's not a TON of reasons to not just use std::vector? or in this, case std::array since the size of chunks can be determined at compile-time?
[/QUOTE]
I think parts of the game development community still reject the standard library/modern C++. Unreal Engine, for example, has it's own set of optimized containers.
Amazingly I still have this (written seven years ago):
[code]
asm( "rjmp .+106" ); // jump to absolute position in executable
[/code]
I was doing some programming on a microprocessor and for some reason the code kept crashing (probably running out of memory or stack space) - I had no good debug tools, but I could see that the code was crashing not long after a return from a function call. Adding an inline assembly jump from the end of function to where the function returned seemed to work so I did it (extra ugly too because I had to compile the code, then check the compiler's output to find the correct offset, then recompile with the new correct offset).
[QUOTE=Helix Snake;52140486]
Have you ever found yourself writing an especially bad line of code?[/QUOTE]
yes.
print("hey guys, scarce here")
Interpret float as int because engine didn't allow me to send it via it's functions
[code]#define WRITE_LONG (*g_engfuncs.pfnWriteLong)
inline void WRITE_FLOAT( float value ) { WRITE_LONG( ( ( *( int * ) &value ) ) ); }[/code]
[QUOTE=suXin;52143015]Interpret float as int because engine didn't allow me to send it via it's functions
[code]#define WRITE_LONG (*g_engfuncs.pfnWriteLong)
inline void WRITE_FLOAT( float value ) { WRITE_LONG( ( ( *( int * ) &value ) ) ); }[/code][/QUOTE]
I remember the way I got around that was to put the float in a union with four bytes then send the bytes individually. But this was is better - three less messages sent.
[QUOTE=suXin;52143015]Interpret float as int because engine didn't allow me to send it via it's functions
[code]#define WRITE_LONG (*g_engfuncs.pfnWriteLong)
inline void WRITE_FLOAT( float value ) { WRITE_LONG( ( ( *( int * ) &value ) ) ); }[/code][/QUOTE]
I would not consider this a bad line of code, it's just a clever little trick to convert to another same-sized structure type, send it and convert back on the other side.
[editline]24th April 2017[/editline]
There are, after all, no side effects and no other obvious/easier way to do it.
[QUOTE=cartman300;52143961]I would not consider this a bad line of code, it's just a clever little trick to convert to another same-sized structure type, send it and convert back on the other side.
[editline]24th April 2017[/editline]
There are, after all, no side effects and no other obvious/easier way to do it.[/QUOTE]
You're right about that, but honestly I remember being really mad at writing this. Without a comment it may confuse someone not proficient with C++, and this hack comes as result from engine not providing the proper way of sending a float.
[editline]24th April 2017[/editline]
I guess I find it worst because of how tricky it is
Can I post code that's not mine, then? At work writing an application to turn 3D models into 3D printer Gcode has involved a lot of peering through other applications that do the same, but Cura has turned up tons of things that really upset me.
To start, there are the conversion macros. Polyclipper is a library used for tons of discrete geometry problems, but it works only in integral units/data types. So, if you have imported model data that's in floats you multiply it by 1000 or some other large number to convert it to an int. Cura does this with these globally defined macros:
[cpp]
#define INT2MM(n) (double(n) / 1000.0)
#define INT2MM2(n) (double(n) / 1000000.0)
#define MM2INT(n) (int64_t((n) * 1000))
#define MM2_2INT(n) (int64_t((n) * 1000000))
#define MM3_2INT(n) (int64_t((n) * 1000000000))
[/cpp]
I guess what upsets me the most about this is using macros for fairly important conversion work - you don't have any guarantees about the datatype that comes out and you pull a c-style cast on the input. Even when I had just started programming C++ this past fall these macros made me feel dirty. I submitted a pull request trying to replace these but like all my PRs to CuraEngine it was rejected.
There's also this conditional based on a GUI option that gets completely disregarded (bracketless if statements ewwww):
[cpp]
if (keep_none_closed)
{
for (PolygonRef polyline : open_polylines)
{
if (polyline.size() > 0)
openPolylines.add(polyline);
}
}
for (PolygonRef polyline : open_polylines)
{
if (polyline.size() > 0)
openPolylines.add(polyline);
}
[/cpp]
Oh, also, PolygonRef is a class that encompasses a reference to a polygon but the "Polygon" and "Polygons" class also inherit from it. [URL="https://github.com/Ultimaker/CuraEngine/blob/master/src/utils/polygon.h"]Look upon the source file and weep[/URL]. Admittedly, I did something a lot like this for my first implementation of the various polygon classes I use. Also, the "ConstPolygonRef" is new as of a month or so ago (iirc), and they've since cleaned this file up considerably but it used to be real bad.
I'd be a lot more charitable about this codebase, too, if they hadn't been so asinine about the PRs and bug reports I'd filed (all bugs found when browsing their code, too).
[editline]edited[/editline]
Actually, I have two lines of code of my own that are rather embarassing and caused an undue amount of work. The first is just this:
[cpp]
const float block_size = 1.0f;
[/cpp]
This doesn't seem terrible, but it broke [I]everything[/I] about texturing in my little voxel engine. Textures seemed really strange and everything flickered and stuttered and the mesh acted really odd in general. Turns out that in most, but not all, of my code related to meshing I just assumed that the size of a block was 0.5. However, the code that placed the vertices used that "block_size" constant, so all my cube voxel faces were 2x the size they should be:
[img]http://i.imgur.com/Fw9XPqa.png[/img]
I spent over a week and a half rewriting various parts of my meshing code to figure out wtf was up, spent ages upon ages trying to see if my texture atlas was broken, and just generally slammed my head into the wall trying to fix this. And it all came down to me setting a constant wrong, and then forgetting about said constant. A painful lesson to learn.
The second was just a silly, but amusing mistake. My first ever successful render of a voxel chunk looked like this:
[t]http://i.imgur.com/9VjzanK.png[/t]
Everything was turned inside out. The faces of my cubes that should be visible weren't visible, and everything that should be invisible was visible. Somehow, I spent several days on this. all it came down to was me calling this with entirely inverted logic:
[cpp]createCube(i, j, k, zNeg, xPos, yNeg, xNeg, yPos, zPos, uv_type);[/cpp]
xNeg, xPos, and so on are six booleans that decide if a cube has to have a face rendered. Since I had inverted the logic, I was rendering everything that should be obscured and not rendering everything that should be visible.
These last two items probably aren't that good of content, but I'm just kinda hoping to see more from this thread before it dies off and wanted to toss something at it i guess
actually this happened recently
[media]https://twitter.com/suxinjke/status/852498168373817345[/media]
Not all mine (I stole it off a forum and improved it a bit), and not a single line, and certainly not the worst thing I've ever seen, but here's a fairly aggravating assembler macro I use to print integers at assemble time (because my assembler can't, evidently).
[code]
macro display_num num
{
value = num
pos=1
if value<0
value = -value
display '-'
end if
while pos*10<=value
pos=pos*10
end while
while pos>=1
digit=value/pos
value=value-(digit*pos)
pos=pos/10
display ('0'+digit)
end while
}
[/code]
[QUOTE=suXin;52151077]actually this happened recently
[media]https://twitter.com/suxinjke/status/852498168373817345[/media][/QUOTE]
[URL="http://stackoverflow.com/a/11599810/410020"]That may not be enough.[/URL] (See the second part of the answer.)
i wrote an inventory system before i knew how to use for loops when i was like 10. it was pretty horrific.
[QUOTE=Pat.Lithium;52154932]i wrote an inventory system before i knew how to use for loops when i was like 10. it was pretty horrific.[/QUOTE]
that's actually pretty amazing
Worst line of code ever written? Hold my beer.
[code]
system("CLS");
[/code]
jk, but really I think my technically worst line of code was probably the very first rogue-like engine I coded in its entirety. It wasn't bad, but it was definitely inefficient. I didn't know about any functions other than void functions back then so I did everything pass by reference without returns because I didn't know you could do it any other way. Don't ask how I managed that, idk. Bad tutorials probably.
[code]
db[ id ] = [ db[ id ][ 0 ], db[ id ][ 1 ], db[ id ][ 2 ], db[ id ][ 3 ], db[ id ][ 4 ] + val ];
[/code]
[code]signed int drawSingleTextLineHook(int textureId, int startX, signed int startY,
unsigned int a4, char *string,
signed int maxLength, int color,
int glyphSize, signed int opacity) {
// yolo
if (NEEDS_CLEARLIST_TEXT_POSITION_ADJUST) {
uintptr_t retaddr;
__asm {
push eax
mov eax, [ebp + 4]
mov retaddr, eax
pop eax
}
if (retaddr == gameExeClearlistDrawRet1 ||
retaddr == gameExeClearlistDrawRet2 ||
retaddr == gameExeClearlistDrawRet3 ||
...
[/code]
[sp]arguably the most elegant solution for what I'm doing but[/sp] this is what I point people to when I tell them why they shouldn't hire me
Looking back at some of the first programs I've written, there are a tons of laughable rookie mistakes like this:
[code]
if(somevar == true)
return true;
else if (somevar == false)
return false;
else
return false;
[/code]
Back in the days when I was one of the devs/owners in gmod community we've started rewriting our community control system and had argument which case we should use for variables... This abomination was made...
[code]
ga = {} --gay/pure lua style
GA = ga --1337 men style
GlobalAdmin = ga --Java style
GlObAlAdMiN = ga --%username% asked :D
globalAdmin = ga --C style
globaladmin = ga --other plain lua style
[/code]
Thankfully after a while we have agreed to use 1st option everywhere. :v:
[QUOTE=zoox;52141540]I think parts of the game development community still reject the standard library/modern C++. Unreal Engine, for example, has it's own set of optimized containers.[/QUOTE]
I think every bigger engine has. STL just isn't optimised for gamedev which is fine because it is meant to cover a lot of ground. The fact that the spec is questionable sometimes and some implementations are... Even more questionable in places (looking at you, std::map) and also that exceptions are used as a primary error handling mechanism doesn't help there.
EASTL is a bit more popular though and there's a movement in the standards committee to get some better support for gamedev needs into the standard.
Also here's a horrible piece of code I've written:
[CODE]
template<class T>
T* GetInstance()
{
static T instance;
return &instance ;
}
[/CODE]
Not a single line really but wrong on so many layers. Even ignoring what horrible thing it implements, it's not even a good implementation. Partly because there is no good implementation of this. Still used it way too much.
Sorry, you need to Log In to post a reply to this thread.
