Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
plushpuffin
Jan 10, 2003

Fratercula arctica

Nap Ghost

The Noble Nobbler posted:

They have Unit Tests. That's more than most places.

Commonly heard at my old job (a web company):

"Oh gently caress oh gently caress oh gently caress. The server went down / the login page started throwing exceptions / etc right after we rolled out our new dlls. gently caress gently caress gently caress gently caress gently caress."

No testing or validation or review whatsoever beyond what the programmer felt like doing at the time. The amazing thing is that they service over 50 colleges at this point and have only lost three of them in five years of doing business.

To contribute (I can't remember whether I've posted this before):

quote:

My boss (no programming experience whatsoever before he picked up VB.NET) wrote this code:

Dim x As Integer = a * b * c * d (all integers)

Naturally we started getting integer overflow errors. Here is how he tried to "fix" it (without testing it at all, of course)

Dim x As Integer = (a * b * c * d) / 100000

Another sample of his programming skillz:

quote:

Dim x As Integer = datareader.GetInt32(0)
If CStr(CInt(x.ToString())) <> "10" Then ...

If you're wondering how anyone could gently caress up data types that badly: he literally just typed and used Visual Studio code completion until it compiled.

Adbot
ADBOT LOVES YOU

enki42
Jun 11, 2001
#ATMLIVESMATTER

Put this Nazi-lover on ignore immediately!
At my last job, our entire codebase was littered with CInt(0) or CInt(1) on method calls - you know, just to be extra sure that the compiler doesn't interpret a literal integer as a long or float or something.

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.

Calipark
Feb 1, 2008

That's cool.
I like reading this thread for "What no to do"(s). But I wish you explained why what you posted is wrong and the correct way they should have done it, more often.

This thread is a good resource for that sort of thing.

Dijkstracula
Mar 18, 2003

You can't spell 'vector field' without me, Professor!

BigRedDot posted:

So I don't work for Lockheed Martin, but I have to work with Lockheed Martin. Let me summarize their approach to testing/debugging:
Remind me to never get into a plane again :gonk:

Blindeye
Sep 22, 2006

I can't believe I kissed you!

Dijkstracula posted:

Remind me to never get into a plane again :gonk:

Don't worry, Boeing and Airbus make our planes, they just make our missile defense, spy satellite, air superiority, nuclear strike capability, and manned space exploration equipment :)

To contribute, I work with computer modelling of buildings and have found that you cannot refine finite plate elements with a certain very widely used program that has been around for decades. Such an oversight means having to basically enter in hundreds of units manually at great time expense. They plan on solving this, to get competitive with other comparable softwares, in version 30.

Avenging Dentist
Oct 1, 2005

oh my god is that a circular saw that does not go in my mouth aaaaagh

Jon93 posted:

I like reading this thread for "What no to do"(s). But I wish you explained why what you posted is wrong and the correct way they should have done it, more often.

This thread is a good resource for that sort of thing.

You can't teach people that way. They'll (you'll) just make equally dubious decisions that are ever-so-slightly different from what's listed here. Everyone's got (at least) 10,000 lines of awful code in them so you just have to get that out of your systyem first.

ColdPie
Jun 9, 2006

Avenging Dentist posted:

You can't teach people that way. They'll (you'll) just make equally dubious decisions that are ever-so-slightly different from what's listed here. Everyone's got (at least) 10,000 lines of awful code in them so you just have to get that out of your systyem first.

Hahaha oh man you should've seen my Tetris clone from high school. Let's just say there were local variables named xxxxx and yyyyy.

Dijkstracula
Mar 18, 2003

You can't spell 'vector field' without me, Professor!

Even though obfuscated code is arguably not an intentional coding horror, I was reminded of this hilarious rock-paper-scissors bot from my AI class a few years back:

code:
/*  Entrant:  Psychic Friends Network   Michael Schatz et al (USA)
              (Unofficial Super-Modified Class, i.e. it cheats)

 note: may cause a Segmentation Fault vs MegaHAL (error: "OUCH")

 > The Psychic Friends Network is a truly hilarious piece of obfuscated C,
 > written by Michael Schatz and company at RST Corporation.  Among other
 > things, it uses an auxiliary function to find good karma, consults
 > horoscopes, cooks spaghetti and (mystic) pizza to go with various kinds
 > of fruit, #defines democrats as communists, and undefines god.  We're
 > still trying to figure out exactly what it is doing with the stack
 > frame, but we do know that it never scores less than +998 in a match,
 > unless it is playing against a meta-meta-cheater.
 To give credit, where credit was due:  Frank Hill and T.J. Walls
 (also of Reliable Software Technologies) were the other minds behind
 Psychic Friends Network.  To see some of their other exploits check out:
 [url]http://www.rstcorp.com/news/gambling.html[/url]
*/
code:
#define RST_ULTIMATE_ANALYZER_FUNCTION rst_ultimate_analyzer_function
#define GOD %
#define democrats communists
/* The open brace was always overrated anyway. */
#define recycle return

/* more readable */
#define THEFUNCTIONSTARTS {
#define THEIFSTARTS {
#define THELOOPSTARTS {
#define spaghetti RST_ULTIMATE_ANALYZER_FUNCTION
#define fresh (
#define rotten (unsigned
#define bananas int)
#define grapes bananas
#define sake 3
#define be_good_for random()

char *find_goodkarma(int *arr) THEFUNCTIONSTARTS
    int turn = arr[0], * karma[50], eleven = 0;
    int magic_bacon;

    *karma = &turn;
    while (!eleven) THELOOPSTARTS

        /* We need to determine the karma rating of the magic bacon. */
        /* This depends highly on the number eleven. */
        for (magic_bacon = 0; magic_bacon < turn; magic_bacon++) THELOOPSTARTS
            eleven = 1;
            if ( (*(*karma + magic_bacon)) != arr[magic_bacon] ) THEIFSTARTS
                eleven = 0;
                break;
            }
        }

        /* why does everyone put their comments at the end of the line? */
        /* is it eleven yet? */             if (eleven) break;
        /* determine karma ratio */         (0[karma]) += (fresh bananas * karma / fresh grapes * karma );
    }

    /* return the . . . whatever the heck this is. */    recycle (char *)*karma;
}

int RST_ULTIMATE_ANALYZER_FUNCTION() THEFUNCTIONSTARTS
    int turn [1];
    int *good_hand, *bad_hand, *pizza;
    int cancer, scorpio, libra;
    /* x is actually my all time favorite variable */     int x;
    int i, democrats;
    int (*callback) () = spaghetti;

    (*&cancer) = (scorpio = (libra = (int)NULL));
        *turn = opp_history[0];

        if (*turn < trials - 2) return libra ? callback() : be_good_for GOD sake;

    /* Consult appropriate astrological signs in order to determine exactly */
    /* which hand to throw. */
    good_hand = (int *)find_goodkarma(my_history);
    bad_hand = (int *)find_goodkarma(opp_history);

    if( cancer == 1 ) return *(int *)"ROCK";
    if( scorpio == 1 ) return *(int *)"SCISSORS";
    if( !(( scorpio != 0 ) || (cancer != 1)) || !((cancer != 0) ||
           (scorpio!=1)) ) return *(int *)"PAPER";
    if( ((  scorpio == 0 ) && (cancer == 1)) || ((cancer == 0) &&
           (scorpio==1)) ) return *(int *)"FONZ";

    /* Process Good Karma Values */
    for (i = 1; i<=*turn; i++) THELOOPSTARTS
        if (good_hand[i] == bad_hand[i]) libra++;
        else if ( ((good_hand[i] - bad_hand[i] +3) GOD sake) == 1) cancer++;
        else scorpio++;

        good_hand[i] = (bad_hand[i] + 1) GOD sake;
    }

    if (bad_hand > good_hand) THEIFSTARTS
        i = cancer;
        cancer = scorpio;
        scorpio = i;
        democrats = 2;
    }
    else democrats = 1;

    pizza = turn;
    for ( x = 0; !( (pizza[0] == libra) &&
        (pizza[1] == scorpio) &&
        (pizza[2] == cancer) ); x++ ) pizza++;

    pizza[0] = 0;
    pizza[1] = 0;
    pizza[2] = 0;
    pizza[3-democrats] = cancer + scorpio + libra;
    democrats = 0;

    recycle (pizza[democrats] + rotten bananas good_hand) GOD sake;
}

# undef GOD
/* Is that possible? */
#undef democrats
#undef recycle
#undef THEFUNCTIONSTARTS
#undef THEIFSTARTS
#undef THELOOPSTARTS
#undef spaghetti
#undef fresh
#undef rotten
#undef bananas
#undef grapes

Dessert Rose
May 17, 2004

awoken in control of a lucid deep dream...

ColdPie posted:

Hahaha oh man you should've seen my Tetris clone from high school. Let's just say there were local variables named xxxxx and yyyyy.

I wish I could find my old IRC library. I created a very bad state machine to parse messages from the server, one byte at a time.

One giant while loop with a switch(state) and case 3: if (byte == ':') then state = 4; etc...

Oh and I thought I was being "cute" with monty python quotes in the comments.

Masked Pumpkin
May 10, 2008
Reviewing some OLD code a while back, I found myself scratching my head. What was this idiot thinking? Then I realised: It was MY code...:sigh:

Amongst it, and one of the factors that helped me recognise it, was this:

code:
/*Drunk now. Fix later.*/

Inverse Icarus
Dec 4, 2003

I run SyncRPG, and produce original, digital content for the Pathfinder RPG, designed from the ground up to be played online.

Mikey-San posted:

code:
// drunk, fix later

Masked Pumpkin posted:

code:
/*Drunk now. Fix later.*/

:derp:

dancavallaro
Sep 10, 2006
My title sucks
Not necessarily a coding horror, but I bet you've never thought to make a "weirdness handler" for your website:

code:
// Email me if something weird happens
function wierdness($message) { weirdness($message); }
function weirdness($message)
{
    global $CONFIG;
    $message .= print_r($_POST,true);
    $message .= print_r($_GET,true);
    $message .= print_r($_SESSION,true);
    $message .= print_r($_SERVER,true);
    $message .= print_r(debug_backtrace(), true);
    mail('[redacted]', "{$CONFIG['website']} Wierdness", $message);
}

Triple Tech
Jul 28, 2006

So what, are you quitting to join Homo Explosion?
Ehh, that's just a debug function. But if you look real close, you'll see that there's also a wierdness function. Even the subject line of the mail message takes that as the canonical spelling...

dancavallaro
Sep 10, 2006
My title sucks

Triple Tech posted:

Ehh, that's just a debug function.

No I know, I just thought it was amusing that the whole function is centered around "weirdness" instead of "errors" or "bugs" or whatever. Again, not a coding horror, but I just thought it was funny nomenclature and couldn't think of a better place to post it.

Lexical Unit
Sep 16, 2003

This code is now my favorite code:
code:
template<class T>
T ByteSwap(T x)
{
	T r;
	int n = sizeof (T);
	if (n > 8)
		std::cerr << "this is probably an error" << std::endl;
	unsigned char* p = (unsigned char*)&x;
	for (int i = 0; i < n; ++i)
		((unsigned char*)&r)[i] = p[n - 1 - i];
	return r;
}

template<class T>
void swap_data(T* data, int n)
{
	int i;
	switch (sizeof (T))
	{
		case 1: break;
		
		case 2:
			for (i = 0; i < n >> 1; i++)
				*(unsigned short*)(data + i) =
					bswap_16 (*(unsigned short*)(data + i));
			break;
			
		case 4:
			for (i = 0; i < n >> 2; i++)
				*(unsigned*)(data + i) =
					 bswap_32 (*(unsigned*)(data + i));
			break;
			
		case 8:
			for (i = 0; i < n >> 3; i++)
				*(unsigned long long*)(data + i) =
					bswap_64 (*(unsigned long long*)(data + i));
			break;
			
		default:
			for (i = 0; i < n / sizeof (T); i++)
				data[i] = ByteSwap (data[i]);
			break;
	}
}

gibbed
Apr 10, 2006

Lexical Unit posted:

This code is now my favorite code:
code:
template<class T>
T ByteSwap(T x)
{
	T r;
	int n = sizeof (T);
	if (n > 8)
		std::cerr << "this is probably an error" << std::endl;
	unsigned char* p = (unsigned char*)&x;
	for (int i = 0; i < n; ++i)
		((unsigned char*)&r)[i] = p[n - 1 - i];
	return r;
}

template<class T>
void swap_data(T* data, int n)
{
	int i;
	switch (sizeof (T))
	{
		case 1: break;
		
		case 2:
			for (i = 0; i < n >> 1; i++)
				*(unsigned short*)(data + i) =
					bswap_16 (*(unsigned short*)(data + i));
			break;
			
		case 4:
			for (i = 0; i < n >> 2; i++)
				*(unsigned*)(data + i) =
					 bswap_32 (*(unsigned*)(data + i));
			break;
			
		case 8:
			for (i = 0; i < n >> 3; i++)
				*(unsigned long long*)(data + i) =
					bswap_64 (*(unsigned long long*)(data + i));
			break;
			
		default:
			for (i = 0; i < n / sizeof (T); i++)
				data[i] = ByteSwap (data[i]);
			break;
	}
}
Why is this a horror?

WalletBeef
Jun 11, 2005

gibbed posted:

Why is this a horror?

If you don't take one look at this and instantly get scared or nervous like that time when you were in 6th grade and you felt like you were going to poo poo your pants because some huge 8th grader was going to eat you alive ... then you're probably a bad programmer.

Painless
Jan 9, 2005

Turn ons: frogs, small mammals, piles of compost
Turn offs: large birds, pitchforks
See you at the beach!

gibbed posted:

Why is this a horror?

- random, pointless printing to cerr
- creating a T instance makes the byteswap function a lot less generic than it could be
- pointless runtime switching (this will almost certainly get optimized away, though)

among other crimes

Dijkstracula
Mar 18, 2003

You can't spell 'vector field' without me, Professor!

To say nothing of that ridiculous switch statement could be removed (edit: or at least simplified) if the author had known about fls()

Dijkstracula fucked around with this message at 21:03 on Jul 7, 2009

Blotto Skorzany
Nov 7, 2008

He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip
Not a horror, but
code:
#ifdef CONFIG_VIDEO_RETAIN
    ...
static void save_screen(void)
{
    ...
}

static void restore_screen(void)
{
    ...
}
#else
#define save_screen()           ((void)0)
#define restore_screen()        ((void)0)
#endif
(from linux/arch/x86/boot/video.c)

We had a fun time in the channel kicking around why Linus might have chosen the ((void)0) construct.

Avenging Dentist
Oct 1, 2005

oh my god is that a circular saw that does not go in my mouth aaaaagh

Otto Skorzeny posted:

We had a fun time in the channel kicking around why Linus might have chosen the ((void)0) construct.

What do you mean? That's idiomatic C for "do nothing".

Blotto Skorzany
Nov 7, 2008

He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip
Not everybody itc knew that:monocle::pirate:

Lexical Unit
Sep 16, 2003

Painless posted:

among other crimes
In case y'all were curious, here are the lines of code that lead me to inspect the byteswap header file in the first place,
code:
vector<complex<float> > payload;
b.get_payload (payload);
// Do we need to byteswap here??
for (int i = 0; i < payload.size (); i++)
  ByteSwap (payload[i]); // why doesn't this work?!?!
:toot: Yes, those comments were checked in.

That Turkey Story
Mar 30, 2003

Lexical Unit posted:

In case y'all were curious, here are the lines of code that lead me to inspect the byteswap header file in the first place,
code:
vector<complex<float> > payload;
b.get_payload (payload);
// Do we need to byteswap here??
for (int i = 0; i < payload.size (); i++)
  ByteSwap (payload[i]); // why doesn't this work?!?!
:toot: Yes, those comments were checked in.

God dammit people are stupid.

Painless
Jan 9, 2005

Turn ons: frogs, small mammals, piles of compost
Turn offs: large birds, pitchforks
See you at the beach!

Lexical Unit posted:

In case y'all were curious, here are the lines of code that lead me to inspect the byteswap header file in the first place,

A horribly wrong way to use a horribly implemented function and simultaneously ignore the existence of a horribly implemented utility function? Nice.

Mustach
Mar 2, 2003

In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.
code:
HashSet<MutableInt>
edit: Actually, it may have been a HashMap<MutableInt,String>

Mustach fucked around with this message at 22:31 on Jul 7, 2009

Inverse Icarus
Dec 4, 2003

I run SyncRPG, and produce original, digital content for the Pathfinder RPG, designed from the ground up to be played online.

Lexical Unit posted:

:toot: Yes, those comments were checked in.

Funny comments checked into the code could make for an interesting twitter feed. I'm pretty sure the codebase I work on could supply a few hundred.

code:
// what do i do here
code:
// Weird cases I haven't tried
code:
// TODO: Check with Dee, Dee knows
Dee hasn't worked here in almost ten years.

code:
// TODO: I really need to return an array
This method returns a boolean

code:
// TODO: fix this hackery
(4 matches)

code:
// DOES THIS MAKE A DIFFERENCE?
code:
// TODO: Check! How do I select an algorithm??
code:
// Truncate the message if > 1024 chars, will almost never be the case probably

Triple Tech
Jul 28, 2006

So what, are you quitting to join Homo Explosion?
I sympathize with some (not really) of them. Sometimes I write a simple interface and document what's missing. Then I change the interface but ignore the nearby comments. Now the comments sound crazy, referring to things that don't exist but are sort of kind of related.

Tad Naff
Jul 8, 2004

I told you you'd be sorry buying an emoticon, but no, you were hung over. Well look at you now. It's not catching on at all!
:backtowork:
How about using RTL characters as delimiters in query strings? Took me forever to figure out what ר is (it's a Hebrew "resh").

Here's an edited version of a nice simple link to an RSS feed:

http:/example.com/rss?sr=ר9+ר575d01ר63"ר
1">ר24ר18ANDר16ר45ר26anyר21ר8contains
ר7ר54salmonר51ר28ר17ר341ר22ר4910
ר36ר14trueר13ר65trueר66ר46ר47engר
44ר31ר39ר35domainר20ר35topicר20ר35creator
ר20ר35creationdateר20ר35rtypeר20ר35langר
20ר35lccר20ר35jtitleר20ר35facet_tlevelר20ר
29ר67vertitleר68ר67titleר68ר67creatorר68ר
67contributorר68ר67publisherר68ר67creationdateר68
ר40ר50+valueר63"scope%3A(UNI)"+typeר63"local"+ר575d00
ר63"ר0"%2F>ר30ר4&ctx=ר15+ר57%3Acomר
63"ר2">ר3772603C4AA1C6012CF90EEAF14A6CBAD3ר25ר43DEMOר
27ר53GUESTר52<com%3ApdsGroup%2F>ר387720091591968372009791915
ר33ר56444.444.444.444ר55ר48trueר41ר72trueר
73ר12s>ר11NOT+scope%3A("VENDOR")ר6ר5ר10&ver=2_1_4

Especially keen is the behaviour when you try to highlight parts of that...

CanSpice
Jan 12, 2002

GO CANUCKS GO

FeloniousDrunk posted:

Especially keen is the behaviour when you try to highlight parts of that...

Holy poo poo.

Seth Turtle
May 6, 2007

by Tiny Fistpump

FeloniousDrunk posted:

Especially keen is the behaviour when you try to highlight parts of that...

I didn't know text could do that.

gibbed
Apr 10, 2006

Painless posted:

- random, pointless printing to cerr
- creating a T instance makes the byteswap function a lot less generic than it could be
- pointless runtime switching (this will almost certainly get optimized away, though)
This is nitpicking and platform specific, although the cerr thing is stupid, yes. :colbert:

Dijkstracula posted:

To say nothing of that ridiculous switch statement could be removed (edit: or at least simplified) if the author had known about fls()
I would like to see this.

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

floWenoL
Oct 23, 2002

Dijkstracula posted:

To say nothing of that ridiculous switch statement could be removed (edit: or at least simplified) if the author had known about fls()

What is fls()?

Thanks, for some reason the machine I'm on don't have man pages for those. Weird.
vvvv

floWenoL fucked around with this message at 01:54 on Jul 8, 2009

gibbed
Apr 10, 2006

floWenoL posted:

What is fls()?
The fls(), flsl() and flsll() functions find the last bit set in value and return the index of that bit.

Blotto Skorzany
Nov 7, 2008

He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

Dijkstracula posted:

To say nothing of that ridiculous switch statement could be removed (edit: or at least simplified) if the author had known about fls()

Did he specify he was on a BSD? ffs(3) and its extensions are in POSIX.1-2001, but fls(3) and friends are BSD-specific from what I can tell.

e: more than BSD-specific, fls(3) first appeared in FreeBSD 5.3 circa November 2004

Blotto Skorzany fucked around with this message at 02:26 on Jul 8, 2009

haveblue
Aug 15, 2005



Toilet Rascal

Seth Turtle posted:

I didn't know text could do that.

Your text system is trying to decide whether that block reads left-to-right or right-to-left and failing.

Dijkstracula
Mar 18, 2003

You can't spell 'vector field' without me, Professor!

Otto Skorzeny posted:

Did he specify he was on a BSD? ffs(3) and its extensions are in POSIX.1-2001, but fls(3) and friends are BSD-specific from what I can tell.

e: more than BSD-specific, fls(3) first appeared in FreeBSD 5.3 circa November 2004
Ah, interesting, good to know that it's not portable, since I use those functions every now and then (but is pretty trivial to implement oneself). I just checked the man page on OSX and assumed it would be present everywhere.

edit: :smug::respek::v:

BigRedDot
Mar 6, 2008

gibbed posted:

This is nitpicking and platform specific, although the cerr thing is stupid, yes. :colbert:
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.

Adbot
ADBOT LOVES YOU

haveblue
Aug 15, 2005



Toilet Rascal

BigRedDot posted:

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

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

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply