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

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›3 »

BigRedDot: Mar 6, 2008

JoeNotCharles posted:

That's NINE copies of the core "save" code, and EIGHTEEN of the sendFileUpdatedMessage call. And yes, there was one change in one of the copies which I can't account for.

From the other end of the spectrum (bigger code block, but repeated few times) I recall some code from a physicist office mate once that had one 3000 line block of code repeated five times in a row—just with what amounted to a loop iteration variable changing everywhere in each block.

# ¿ Aug 31, 2008 03:17

Adbot: ADBOT LOVES YOU

# ¿ Apr 19, 2024 20:23

BigRedDot: Mar 6, 2008

floWenoL posted:

The person who wrote the code may be completely familiar with the function, but maybe not the person reading the code later.

I pretty fastidiously avoid using reference arguments as return parameters, for exactly this reason. Unfortunately I seem to see them alot at work, both from coworkers and from some of our libraries, for instance:

code:

ProjectDegrees( double northing, double easting, double & lat double & lon );
UnprojectDegrees( double lat, double lon, double & northing, double & easting );

Athough, I hate this pair even more for their names, since I can never remember if, for instance, ProjectDegrees is supposed to signify Project_TO_Degrees or Project_FROM_Degrees and I invariably have to go look which is which. There were lots of function names with this problem at a previous job, grrr. But I digress. . .

Anyway I did just recently finally concede a situation where reference return parameters were more useful than not (to me anyway), so I can't make a blanket admonition. I got tired of writing the the little snippets of code to read values out of text entry widgets all over the place, and having a stable of separate GetFloat, GetUnsigned, GetDouble, GetChar, GetInt, GetWhatever, ... functions annoys the hell out of me (other people at work do this in other situations, and that's also a coding horror to me). So I use:

code:

template <typename T>
bool ScrapValueFromWidget( Widget * w 
                           T &      val ) { ... }

If only return types were part of function signatures in C++ I could make the return type T and just leak the exception from boost::lexical_cast if it fails, but we go to work with the compiler standard we have, not the compiler standard we might want. In any case, I think the function name makes it pretty clear, and it's documented as a return parameter in the doxygen docs for anyone that cares.

# ¿ Sep 1, 2008 18:26

BigRedDot: Mar 6, 2008

floWenoL posted:

I don't get why you can't use T *val here.

There's absolutely never a case where NULL should be passed in, but allowing T * would mean the function now has to check that in every single case. That's just silly, and I don't want to do it.

# ¿ Sep 2, 2008 01:06

BigRedDot: Mar 6, 2008

floWenoL posted:

No it doesn't. That's silly.

Look, I'd love to have explicitly labeled output parameters too. And in the absence of such, I almost always think it's better to not use reference parameters as output parameters. But I'm not going to uphold a convention "just because" in the cases where it doesn't make sense.

If I allow T* then what are the cases. I can 1) never check for NULL, and fail ungraciously if someone happens to pass it in, and I try to write to it. That just won't fly, at least not on the product I develop. So I am left with 2) checking for NULL every single time. Apart from just being irksome in the "why do I have to do this completely unnecessary thing" sense, it's also not so good in that although I can return failure, the real reason for the failure (a NULL pointer that should have never been passed in, as opposed to a failure of lexical_cast) is not immediately evident.

Or, in a trivial function with a blatantly obvious name and explicit supporting documentation I can allow a reference output parameter and not have to worry about any of the above because it's not even possible in the first place.

# ¿ Sep 2, 2008 01:28

BigRedDot: Mar 6, 2008

Seriously, what is attempting to iterate past the end of a sequence, if not an exceptional circumstance?

# ¿ Nov 8, 2008 18:06

BigRedDot: Mar 6, 2008

You forgot to mention that it took three people to craft that gem.

# ¿ Feb 27, 2009 22:59

BigRedDot: Mar 6, 2008

quote:

An attacker could, for example, use it to download privilege escalating malware (one of the many reasons your server should never execute wget...)

Runs on an airgapped network, so downloading malware or anything else for that matter, is not a concern. That's not an affirmative defense of this code, of course.

# ¿ Feb 28, 2009 07:56

BigRedDot: Mar 6, 2008

bouyancy posted:

Exactly but what is worse is that in Java the ResultSet throws an exception if you try to read from a column that wasn't selected in the query. So if you use this on a query that only selects a couple columns then it will throw and catch about three hundred exceptions for each row returned.

That is some sloppy laziness right there. Seriously, with some hard work and dedication you ought to be able to improve that number to three thousand exceptions per row! Come on, get to it!

# ¿ Apr 1, 2009 16:26

BigRedDot: Mar 6, 2008

In all fairness though, one person who wrote some of that code will freely admit that he is not a programmer, and that he doesn't particularly want to program or know why he is occasionally tasked to do so when it's not really part of his job.

The real horror is that we hire so many non-programmers to do programming.

BigRedDot fucked around with this message at 05:51 on Jun 3, 2009

# ¿ Jun 3, 2009 05:48

BigRedDot: Mar 6, 2008

Inverse Icarus posted:

I've heard a lot of this from people who don't work for high-tech companies. A lot of non-CS and non-Math majors end up coding, and that scares the hell out of me.

Thankfully my job's not like that.

We work in a research lab. The physicists and such mostly use matlab (or python/numpy/scipy if they are feeling frisky) for prototyping and algorithm development. For whatever reason, we hire lots of EE fresh-outs and other people without much relevant programming experience to do coding for production systems on billion-dollar platforms.

BigRedDot fucked around with this message at 06:28 on Jun 3, 2009

# ¿ Jun 3, 2009 06:26

BigRedDot: Mar 6, 2008

Flobbster posted:

I don't think I've ever seen more obvious proof that Larry Wall is just trolling the entire programming language community with Perl. There's no other explanation for the poo poo that goes on there.

I completely agree, but this was the proof for me:

code:

bryan@Laptop-2 ~ $ perl -e 'print 3*undef; print "\n"'
0

As you can see Larry Wall is trolling all of mathematics, as well. Actually, the fact that perl used to default to dynamic scoping in the past was what clued me in.

# ¿ Jun 15, 2009 19:15

BigRedDot: Mar 6, 2008

plushpuffin posted:

No testing or validation or review whatsoever beyond what the programmer felt like doing at the time.

So I don't work for Lockheed Martin, but I have to work with Lockheed Martin. Let me summarize their approach to testing/debugging:

quote:

Reboot until it sort of works, then the problem never existed.

# ¿ Jul 5, 2009 09:25

BigRedDot: Mar 6, 2008

gibbed posted:

This is nitpicking and platform specific, although the cerr thing is stupid, yes.
I would like to see this.

It seemed mostly OK to me, although I wouldn't make it a template.

It almost certainly doesn't work as intended for floating point variables, yet it accepts them silently.

# ¿ Jul 8, 2009 03:19

BigRedDot: Mar 6, 2008

sex offendin Link posted:

There is no generalized way to byte-swap IEEE 754 numbers anywhere, they are for all intents and purposes non-portable.

That was my point?

# ¿ Jul 8, 2009 03:46

BigRedDot: Mar 6, 2008

RussianManiac posted:

what C compiler doesn't support function prototypes, and how would that macro help? Do you mean so they can change it to a function pointer?

Old, old, old pre-ansi "K&R" compilers. And no it doesn't change it to a function pointer. The macro would be redefined as

code:

#define __(a) ( )

You know, so if your compiler doesn't support prototypes, you won't give it one. Although I seem to recall always seeing "P_(a)" from cproto as the conventional spelling, not "__(a)"

# ¿ Jul 9, 2009 06:55

BigRedDot: Mar 6, 2008

sensual donkey punching posted:

code like this gives me great pleasure. reminds me of the ever popular pastime of writing unnecessary forth interpreters

The Sun PROM Monitor comes to mind.

# ¿ Jul 10, 2009 04:41

BigRedDot: Mar 6, 2008

Lexical Unit posted:

Haha, same file:
code:
#define SIN(x) (sin((x)*M_PI/180))
Note that SIN() is only ever called once in the file, and it's never #undefed.

We both know this particular developer's sins can never be erased.

# ¿ Jul 23, 2009 21:58

BigRedDot: Mar 6, 2008

Dijkstracula posted:

s/business//

I work on sonar systems for submarines, I can say confidently that regular expressions are not at all an integral part of the programming we do.

# ¿ Aug 1, 2009 18:16

BigRedDot: Mar 6, 2008

Please you've got to stop. Between that and the find you wrote up on my whiteboard earlier today, I am going to lose all hope.

# ¿ Aug 19, 2009 21:16

BigRedDot: Mar 6, 2008

Lexical Unit posted:

So that pretty much describes my boss

Which is especially ironic considering the utter shite that he produces. Having to use, interface to, or otherwise deal with his code is like a millstone about my neck.

BigRedDot fucked around with this message at 20:10 on Sep 3, 2009

# ¿ Sep 3, 2009 20:07

BigRedDot: Mar 6, 2008

Zombywuf posted:

This is the closest I've seen to managing to parody timecube. However it still falls short. Is it actually possible to parody it?

Timecube is invariant under parody operations.

# ¿ Sep 6, 2009 14:03

BigRedDot: Mar 6, 2008

Lexical Unit posted:

code:

int sign = (1, 0, -1);
float f = /* ... */;
float f_neg = (f == fabs (f)) ? 1.0 : -1.0;
float some_calc = fabs (f * /* ... */);

I suppose I should take comfort knowing that this individual no longer works here, OTOH they left because they got into CS grad school. :aaaaa:

I wonder what clever thing they thought they were accomplishing with "int sign = (1, 0, -1);"

BigRedDot fucked around with this message at 15:49 on Sep 29, 2009

# ¿ Sep 29, 2009 15:46

BigRedDot: Mar 6, 2008

Fecotourist posted:

Simulated annealing? By any chance did the assembler documentation mention a "temperature schedule"?

I fail to believe anyone would have done this except perhaps out of spite or as an attempt at humor.

# ¿ Nov 5, 2009 18:59

BigRedDot: Mar 6, 2008

spotted today:

code:

foo.cc:100: error: no matching function call to 'CalcWhatever(float*&, float*&, float, float&, int&, int&, int&, float&, 
float&, float&, float&, float&, float&, float&, float&, std::vector<float, std::allocator<float> >&, 
std::vector<float, std::allocator<float> >&, float&, float&, float&, float&, int&, float&, float&, int&, float&, 
float&, float&, float&, float&, unaigned int&, unsigned int&, float&, int&)'

# ¿ Nov 6, 2009 20:29

BigRedDot: Mar 6, 2008

Mr Dog posted:

O(1/2 n)

LOL Except those optimized versions that run in O(1/2) !

Also an unaigned int is what you get when you have to type that much crap by hand because the dev box is airgapped.

# ¿ Nov 7, 2009 00:57

BigRedDot: Mar 6, 2008

Mustach posted:

So maybe there's a barrel underneath this barrel.

I'm sorry no one told you before now, but it's barrels all the way down.

# ¿ Nov 10, 2009 15:23

BigRedDot: Mar 6, 2008

I guess we've just run out of coding horrors then?

# ¿ Nov 29, 2009 18:44

BigRedDot: Mar 6, 2008

Mustach posted:

There is a 100% chance that if someone structures their code like that, they are sick in the head.

That's just stupid. I've worked at places that had coding standards that mandated SESE. Someone else having a stupid policy that I have to live with doesn't make me sick, or a bad programmer.

# ¿ Dec 2, 2009 16:04

BigRedDot: Mar 6, 2008

Fib posted:

This is a little gem of a horror

Your tax dollars buy only the best.

# ¿ Dec 10, 2009 17:38

BigRedDot: Mar 6, 2008

RussianManiac posted:

Is it possible to pass code that is not either of those 3 values? could you pass an arbitrary integer instead of something of type Code?

Sure, you can cast some other integral type to type Code.

# ¿ Jan 6, 2010 22:23

BigRedDot: Mar 6, 2008

GrumpyDoctor posted:

I get a kind of goofy future-proofing vibe from it, as though the author was worried about people adding other possible Code values but wanted to make sure this component ignored them.

I see it in code where I work, but we often cast integral values that are gotten over sockets or read from hardware registers into enums. It's a "just in case the sender can't be trusted" sort of thing. Of course that code is still a horror, since the switch is worthless.

# ¿ Jan 7, 2010 01:07

BigRedDot: Mar 6, 2008

TRex EaterofCars posted:

That has to be programming by permutation. I refuse to believe anyone deliberately writes code like that.

Demorgan's Law is a law goddamnit, and you better follow it!

# ¿ Jan 25, 2010 19:49

BigRedDot: Mar 6, 2008

Janin posted:

Where I work, none of the builds use -Wall because "it prints too much noise".

This is true for most of our code as well. By the way we write software for submarines.

# ¿ Feb 8, 2010 15:00

BigRedDot: Mar 6, 2008

Avenging Dentist posted:

Get a Buildbot and a person who hates the color orange and gradually add -Whatever options to the Buildbot.

Would that I could. Our three-sided wheel, homespun, messy perl-script build system is a horror unto itself. Why yes, it is named after the person who wrote it.

Edit: zombywuf I want to hug you

# ¿ Feb 8, 2010 15:52

BigRedDot: Mar 6, 2008

Avenging Dentist posted:

Could be worse. You could be using autotools.

I've used autotools, so I know just how much it sucks. I can state without hesitation that this home-grown abortion is worse. My own preference would be cmake, but if the choice was autotools or what we have now I would switch in a heartbeat. Just to give you an idea: in order to have a "local" configuration that gets built here in our limited environment, and a "production" configuration that gets built at the lead integrator's site in the full environment, every one of our entire huge trees, in every one of our many baselines, is duplicated in its entirety, so that basically a symlink can change. Make sure you only, only ever pull from prod to local! Never the other way! No, really, not joking.

BigRedDot fucked around with this message at 19:41 on Feb 8, 2010

# ¿ Feb 8, 2010 19:35

BigRedDot: Mar 6, 2008

ZorbaTHut posted:

I was working with an annoying C library a while back that defined its own FALSE and TRUE constants. I wanted to use those names on my own, so I ended up with
code:
#ifndef TRUE
#define TRUE 1
#endif
Then I copy-pasted it for FALSE. Except I forgot to change the 1 to a 0.

That one took me a while to find, let me tell you.

LOL I am reminded of the old, but still humorous How to Write Unmaintainable Code

quote:

Another technique is to make TRUE and FALSE have the same value, though most would consider that out and out cheating. Using values 1 and 2 or -1 and 0 is a more subtle way to trip people up and still look respectable.

# ¿ Feb 10, 2010 14:46

BigRedDot: Mar 6, 2008

Zombywuf posted:

So yeah, what time it actually is at the time of the event you're interested in.

And if what you are interested in is the price of futures contracts on hypo-widgets last Tuesday, when they weren't being traded because the New Hollandia exchange was closed for St. Smibbin's day?

That's the major complication with financial dealings. What you want it is accurate time series data with no drop-outs. But that's not what the real world provides. Sometimes the event you care about (a price usually), at the time you are interested, simply does not exist. Financial data is very, very dirty. By contrast, your planes always land, one way, or another.

# ¿ Feb 15, 2010 14:53

BigRedDot: Mar 6, 2008

The person that wrote this has been fired, sure, but that's meager consolation when I stumble across all the droppings he left. This was buried under a few levels of indentation, in the middle of a giant switch, in a 900 line function. I've preserved the formatting also for your enjoyment.

code:

    vector<Thing> filteredThings;
    filteredThings.clear();
    for(int i=0 ; i < someVector.size(); i++)
    {
        for(int j=0; j < mStupidNamingConvention.size(); j++ )
        {
            if((mStupidNamingConvention[j].GetAThing().GetValue("FooID")) == someVector[i].GetValue("FooID"))
            {
                filteredThings.push_back(someVector[i]);
                j = mStupidNamingConvention.size()+1;
            }
        }
    }

I think a break statement would have sufficed....

BigRedDot fucked around with this message at 00:33 on Mar 30, 2010

# ¿ Mar 29, 2010 21:40

BigRedDot: Mar 6, 2008

More irredeemable code today (approximated):

code:

struct Thing {
   double foo;
   // ... 
};

class ThingContainer {
    int ContainerSize();
    string getID(int index);
    int GetFoo(int index);
    double GetBar(int index);

private:
   map<string, vector<Thing>> mIDToThings;
   map<string, vector<Thing>>::iterator ThingIt;

};

int ThingContainer::ContainerSize() 
{
    return (int)mIDToThings.size();
}

string ThingContainer::GetID(int index) 
{
   ThingIt=mIDToThings.begin();
   for(int i=0); i<index; i++)
      ThingIt++
   return ThingIt->first;
}

// This method was repeated verbatim for six or seven other fields in Thing
// It might be nice if the name mentioned something about computing an average
double ThingContainer::GetFoo(int index) 
{
   ThingIt=mIDToThings.begin();
   for(int i=0); i<index; i++)
      ThingIt++
   double sum = 0;
   for(int j=0; j<ThingIt->second.size(); j++)
   {
      sum+= ThingIt->second[j].foo;
   }
   return sum/ThingIt->second.size();
}

This was then used like:

code:

for(int i=0; i<mThingContainer.ContainerSize(); i++ )
{
    mTargetContainer.GetID(i); 
    mTargetContainer.GetFoo(i);  
    // six or seven more, one for each field in Thing
}

So yes, he's indexing a map like a vector and turning what should be a single log(n) lookup into a nice STL map into seven or eight linear lookups, every time. No, he never actually uses the map key except when iterating over the map by hand in a for loop.

BTW if you are a US citizen your tax dollars paid for this.

# ¿ Mar 31, 2010 20:40

Adbot: ADBOT LOVES YOU

# ¿ Apr 19, 2024 20:23

BigRedDot: Mar 6, 2008

Otto Skorzeny posted:

(from the source code of pidgin, in gtkimhtml.c)

from here, there are naught but coding horrors.

Also, the pidgin devs are arrogant dicks.

# ¿ Apr 11, 2010 07:25

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›3 »