Coding Horrors: You can gather all your technical debt into one easy framework!

Zakalwe · # ¿ Jun 14, 2008 16:35

From a test I recently corrected.

__m128 y = _mm_mul_ps(x, _mm_set1_ps(1.0f));

This is an SSE y = x*1;

__m128 rcp = _mm_rcp_ps(_mm_set1_ps(1.0f));

rcp = 1/1;

Wait, there's worse

__m128 rcp = rcp_nr(_mm_set1_ps(1.0f));

Hrmm. SSE reciprocal is not very accurate. I know, let's use a newtown rhapson iteration to increase the accuracy of our 1/1 calculation!

Adbot · # ¿ Apr 25, 2024 12:15

Zakalwe · # ¿ Jun 15, 2008 01:26

ZorbaTHut posted:

I wrote a 3000-line function once. It included a "goto".

3000 lines is :psyduck:

, but there's nothing wrong with using the occasional goto. "Never ever use goto" is generally promulgated by people who've heard of Dijkstra's article but never read the drat thing.

Zakalwe · # ¿ Jun 15, 2008 01:51

ehnus posted:

Two that come immediately to mind are breaking out of nested loops in C/C++/C# and cleaning up/freeing resources upon failure in environments that do not provide exceptions. I've used them when I've written parsers too as there was no other clear way of providing similar functionality.

DING DING DING.

Zakalwe · # ¿ Jun 15, 2008 02:27

Ugg boots posted:

The one thing I did like about PHP was that they offer the break X construct that lets you break out of nested loops as far as you want.

Until you add another loop and WHOOPS, you're not where you think you should be.

Zakalwe · # ¿ Jun 15, 2008 14:20

floWenoL posted:

Here's a nickel, kid, go buy yourself a C book.

Modern (non-lovely *cough*) languages like Java and Perl provide labeled loops and ways to break out of them, so that removes one of the two remaining use cases for gotos nowadays, and exceptions remove the other one.

break my_label is no different than goto my_label as far as breaking out of loops is concerned. It just removes the functionality of one "evil" and sticks it in another.

Zakalwe · # ¿ Jun 15, 2008 14:58

zootm posted:

This is largely correct but the essential point here is that goto should be limited to the point where execution paths can be easily determined. Gotos within local scope for breaking out of loops and the like are theoretically fine, but jumping between functions, jumping to arbitrary points to emulate flow control, and so on is craziness. There's definitely valid uses within C from what I've seen, though.

Agreed.

Here's another coding horror. A student of mine refused to use a for loop. He wrote everything as a while loop

for(int i=0; i<5; ++i){
do_stuff(i);
}

was

int i=0;
while(i<5){
do_stuff(i);
++i;
}

Now they'll compile down to the same code 99.99% of the time, but it takes up more space, reads strangely to anyone else and the iterator now has scope outside of the loop. I never got a satisfactory answer from him as to why he did this.

Zakalwe · # ¿ Jun 15, 2008 22:46

3rd year college level CS course, not a general programming course I'm afraid. The kid was very bright, but I got an air of "self-taught programmer" from him which would probably explain why he did things that way. The same course, although not the same person, yielded the above SIMD 1/1 and y=x*1 code snippets.

I know I've been guilty of the same thing. When learning C years ago (self taught) I had a habit of using (*foo).bar rather than foo->bar which some people would call horrific.

Zakalwe · # ¿ Sep 2, 2008 03:33

boost tuples http://www.boost.org/doc/libs/1_36_0/libs/tuple/doc/tuple_users_guide.html

TR1 has tuples, but that poo poo isn't really official of course. Roll on 0x

Zakalwe · # ¿ Nov 26, 2008 20:02

A colleague and I were looking through a ray-tracer that someone released as we were implementing one of our own algorithms in it in order to see if the gains we were seeing were peculair to our own code.

We came across a piece of code like this...

code:

for( int axis = 0; axis < 3; axis++){
       switch(axis){
              case 0 : do_stuff(axis); break;
              case 1 : do_stuff(axis); break;
              case 2 : do_stuff(axis); break;
       }
}

Zakalwe · # ¿ Jan 4, 2009 22:09

beer_war posted:

code:

bool some_function()
{
 //...

 return boolean_expression ? true : false ;
}

That's a shooting offense. If you know enough to use the ternary operator then I expect you not to write code like that.

Students that I give grinds to have a habit of doing this when they come to me first. I beat it out of them.

code:

bool r = foo();

if(r == true){
    return true;
}
else {
    return false;
}

An old student of mine writes code like this:

code:

int i = 0;

while(i< 10){
    blah blah blah.

    i++;
}

He refuses to use for loops for any reason. He's the epitome of self-taught programmer. Never showed up for the class I taught and despite thinking he was the best in the class, ended up in the bottom half.

Zakalwe · # ¿ Jan 5, 2009 02:27

ih8ualot posted:

I know I'm in the minority here, but I find
just fine. I can read the if line easily, and it makes sense.

<SNIP>

And like I said, I know I'm probably in the minority.

I'm with you, but perhaps for different reasons. If it's a boolean expression I don't use == true. If however the expression would rely on integral promotion to be boolean I'll use a comparison like the following

code:

bool is_odd(const int a)
{
        return( a%2 == 0);
}

The == 0 is strictly not necessary, but it's how I code

The same goes for checking pointers. I always use

code:

if(p == NULL)

rather than

code:

if(!p)

All of these things are pretty stylistic though. It does enhance code readability to a certain extent. Of course there is always the problem that it's slightly more likely that you'll use an assignment operator where you meant to use a comparison
(= vs ==)

Zakalwe · # ¿ Jan 5, 2009 03:57

*Whoops* was trying to think of a simple case where I could drop a comparison. You get my point though

replace 1 with 0 on that code. Changing it to is_even means you can't just drop the comp.

Zakalwe · # ¿ Jan 7, 2009 17:26

minato posted:

code:

float InvSqrt (float x){
    float xhalf = 0.5f*x;
    int i = *(int*)&x;
    i = 0x5f3759df - (i>>1);
    x = *(float*)&i;
    x = x*(1.5f - xhalf*x*x);
    return x;
}

gently caress yoooooooooouuuuuu.

That's not a horror, that's an extreme optimisation for an operation done a lot in many renderers. Low accuracy bitwise hack combined with one Newton-Raphson iteration.

Zakalwe · # ¿ Jan 7, 2009 19:12

ehnus posted:

It's a coding horror for any x86 (post-1999) or PowerPC based computer.

In the context in which it was originally written it's not a coding horror which is what counts. Today of course I would (and do) use SSE for such things.

Zakalwe · # ¿ Jan 7, 2009 19:43

heeen posted:

Can you elaborate on the why and how you do things nowadays?

My research is on RTRT (real-time ray-tracing). Packet tracing (Google Keywords: Ingo Wald ICRT Packet Tracing) at its simplest involves tracing 4 rays at a time in SSE. Other related techniques such as MLRTA, frustum culling etc. all benefit from using SSE.

Here's a reciprocal SSE sqrt from my code.

code:

_m128 rsqrt_nr(const __m128 &a)
{
    const __m128 half = _mm_set1_ps(0.5f);
    const __m128 three = _mm_set1_ps(3.0f);
    const __m128 r = _mm_rsqrt_ps(a);
    return _mm_mul_ps(_mm_mul_ps(half, r),
           _mm_sub_ps(three,
           _mm_mul_ps(_mm_mul_ps(a, r), Ra0)));
}

I perform a simple NR on the _mm_rsqrt_ps intrinsic to gain about 22 bits of accuracy (from the original 12)

Zakalwe · # ¿ Jan 7, 2009 19:55

btw, here's something I found out recently.

Here's that line from the above code to access the bits in a float.

code:

 int i = *(int*)&x;

I used to do that too, but you can also do this in C++

code:

int i = (int&)x;

Perhaps a bit off-topic for a coding horrors thread.

Zakalwe · # ¿ Jan 7, 2009 19:59

Sartak posted:

Nope I'd say you're in the right place.

Optimisation when relevant is not a coding horror.

Zakalwe · # ¿ Jan 7, 2009 20:02

hexadecimal posted:

Can you explain why this is faster than doing int i = (int)x

Uhh because what you posted doesn't work?

Zakalwe · # ¿ Jan 7, 2009 20:08

dancavallaro posted:

I'm not a big C guy, so I'm kind of guessing here, but just doing (int)x will cast x to an integer, and chop off anything after the decimal. But (int&)x or *(int*)&x preserves all of the bits of the float, but as an integer. This is what you need, because the point is to be able to do bit operations on the bits of the float.

correct

Zakalwe · # ¿ Jan 7, 2009 20:12

hexadecimal posted:

I just tried it and it did

It compiles, but it doesn't do what we want to do (access the bits in a float). Taking 5 to get some food sounds like a good idea.

Zakalwe · # ¿ Jan 7, 2009 20:41

Ugg boots posted:

Where in the C standard does it specify that unioned elements will occupy the same space in memory?

Wait, don't unions by definition start at the same point in memory. If the types have the same length they completely overlap each other. Am I missing something here?
Edit: Wagammama's ramen is delicious.

Zakalwe · # ¿ Jan 17, 2009 02:48

Wait isn't that a correct fix for erasing based on an iterator so you don't invalidate the iterator itself? forgive me if I'm wrong here as it's nearly 2am

Zakalwe · # ¿ Jan 17, 2009 04:30

Otto Skorzeny posted:

Did you ever get your drat Anschuetz working

Yeah the last 200 rounds I ran through her popped out nicely

I was going to go down to the range tonight and put a box or two of Eley Sport through her, but I got stuck late fixing some code for a paper.

on topic: from a colleague's code earlier on today

//This should not be an odd number!
const int static num_photons = 53;

Zakalwe · # ¿ Jan 21, 2009 23:41

shrughes posted:

Everything on this page: http://www.xsharp.org/samples/

Jesus Christ. I don't know what to say. The site's too detailed to be a joke.

The most hilaroius thing is the tag-line "Code is poetry". Yes, and X# is Jabberwocky

Zakalwe · # ¿ Feb 5, 2009 03:48

One time pads are bad how :confused:

edit: Whoops, missed the "encrypt passwords" bit

Zakalwe fucked around with this message at 03:52 on Feb 5, 2009

Zakalwe · # ¿ Feb 13, 2009 00:47

GrumpyDoctor posted:

check out this sweet container (DynamicArray is a homegrown resizable array that's basically std::vector without bounds checking and the added bonus of being completely inscrutable to the visual c++ debugger)

home-grown wheel reinvention, difficult to debug, the lack of reference parameters/returns and an always O(n^2) bubble sort.

Beautiful

Zakalwe · # ¿ Apr 24, 2009 20:51

Captain Cool posted:

Well, it means you're using public class members

Not necessarily a coding horror. Full encapsulation doesn't always make sense in the context of the problem.

I once had a friend who returned non-const references in getters argue against public class members. :psyduck:

Zakalwe · # ¿ Jun 26, 2009 18:49

From the Bullet SDK User Manual

quote:

Often it is necessary to maintain an array of objects. Originally the Bullet library used a STL std::vector data structure for arrays, but for portability and compatibility reasons we switched to our own array class.

Zakalwe · # ¿ Jul 1, 2009 00:16

Vinterstum posted:

Looks like it was for Visual Studio compatibility: http://bulletphysics.com/Bullet/phpBB3/viewtopic.php?f=9&t=3638&p=13749&hilit=std%3A%3Avector#p13749

Oh the HORROR!

I have a really hard time believing that VS2008 has a broken implementation of something as pervasive in C++ code as std::vector.

Instead I'm inclined to believe they're doing something dumb and out of spec that works in GCC and therefore "VS2008 is broken".

Bullet's Demos are also the most convoluted pieces of code. Whole bunches of similar classes are constructed via Macros. Each demo inherits from a huge Demo class. Each demo shares random files with other demos. Heaven forbid examples and demos are easy to understand.

Another WTF: The multithreaded implementation was first realised on the Cell. The more generic Linux/Windows code is a *direct* port of this. Each worker thread operates with a 256k local store that it has to populate via memcopy() from the main pool :psyduck:

Zakalwe · # ¿ Jul 2, 2009 01:37

Vinterstum posted:

Something dumb like wanting some of their internal data types to be 16 byte aligned, which the VS STL implementation can't handle (We actually had problems with this at work as well, I remember).

Again: Oh, the horror!

They claimed VS was buggy. This is not a bug and therefore std::vector in Visual Studio's STL implementation is not broken in this regard. Perhaps the real WTF is the cheap LOL MS shot for no other reason than VS is not GCC.

Zakalwe · # ¿ Jul 27, 2009 03:37

mr_jim posted:

Why?

code:

if (b == null) goto CLEANUP;

// Do something useful

CLEANUP:

// Cleanup

How is that wrong?

It's not. Some people have "OMG goto is evil" beaten into them by idiots who should know better.

Zakalwe · # ¿ Aug 7, 2009 21:08

fletcher posted:

I can't imagine why they even put something like that in. Are people really that lazy that they can't keep their code formatted properly as they write it?

I spend most of my "coding time" thinking through algorithms or optimizing code. Typing is the tedious part. I've set up my indenter to format my code in the style I like. I use autocomplete. It's a tool. Do you use an IDE?

Zakalwe · # ¿ Aug 7, 2009 21:28

fletcher posted:

Yeah, I use autocomplete all the time, I couldn't live without it. I have my indenter setup the way I like as well, and it will add curly braces for me at convenient times. None of that has to do with what I was talking about. I use eclipse.

Sorry dude, read you completely wrong. Thought you were pulling out that "real programmers do X" bullshit.

Zakalwe · # ¿ Aug 11, 2009 20:57

Lexical Unit posted:

an unholy abomination.

Jesus H Christ. I think I need to lie down for a minute.

Zakalwe · # ¿ Aug 19, 2009 18:47

ryanmfw posted:

Am I just a bad programmer or is something like this bad:
code:
//Read the value of the analog input from a data acquisition device
output(i) = device_read(i);
I know it works but that, and the fact that inputs are outputs and outputs are inputs keeps throwing me off constantly. I won't say I'm good or anything but really?

output(i) returns a reference to the i'th element of an internal array I'd guess. If so I'd personally overload[] and use output[i] = device_read(i) myself; It's a stylistic choice.

Zakalwe · # ¿ Oct 7, 2009 04:36

yaoi prophet posted:

oh god my eyes

edited to not break tables, the original has no linebreaks

(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])

Zakalwe · # ¿ Oct 7, 2009 04:52

yaoi prophet posted:

vvvv is that the regex for matching all valid e-mail addresses?

Ding ding ding.

Zakalwe · # ¿ Oct 7, 2009 04:59

Janin posted:

Why is it so much shorter than Mail::RFC822::Address?

Different RFC. I posted RFC2822

Adbot · # ¿ Apr 25, 2024 12:15

Zakalwe · # ¿ Jun 19, 2010 02:33

shrughes posted:

Especially since a feature has been built into C99 for this very purpose.

Perhaps he means the zero size data array? It's a trick used in a lot of header structs where the data size is variable. The actual size of the data member is actually 0, but of course using it as an array allows you to access the data past the header.