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!

«‹›1503 »

Wuhao: Apr 22, 2002; Pimpin' Lenin

Lexical Unit posted:

Roman numeral horror

I am going to have nightmares about seeing this somewhere in 5 years from someone who believes in proactive job security.

edit: no good reason for me to have quoted all that

Wuhao fucked around with this message at 05:47 on Aug 12, 2009

# ? Aug 12, 2009 05:29

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 19:56

Avenging Dentist: Oct 1, 2005; oh my god is that a circular saw that does not go in my mouth aaaaagh

Wuhao posted:

I am going to have nightmares about seeing this somewhere in 5 years from someone who believes in proactive job security.

And just think, you can actually do practical stuff that looks more-or-less like that. :c00l:

# ? Aug 12, 2009 05:31

tripwire: Nov 19, 2004; _{ghost flow}

Avenging Dentist posted:

And just think, you can actually do practical stuff that looks more-or-less like that.

Please don't tell me that at this point in time

# ? Aug 12, 2009 05:33

Avenging Dentist: Oct 1, 2005; oh my god is that a circular saw that does not go in my mouth aaaaagh

tripwire posted:

Please don't tell me that at this point in time

Here's some code I wrote for a memoizer for C++0x:

code:

namespace detail
{
    template<typename T>
    struct remove_const_ref :
        std::remove_const<
            typename std::remove_reference<T>::type
        >
    {};

    template<typename T,typename U>
    struct is_similar : 
        std::is_same<
            typename detail::remove_const_ref<T>::type,
            typename detail::remove_const_ref<U>::type
        >
    {};
}

template <typename T,typename Ret,typename ...Args>
class memoizer_base<T,Ret (Args...)>
{
public:
    // ...

    template<typename ...CallArgs>
    result_reference operator() (CallArgs &&...params)
    {
        return call<
            typename boost::mpl::if_<
                typename detail::is_similar<CallArgs,Args>::type,
                CallArgs&&,
                typename detail::remove_const_ref<Args>::type&&
            >::type...
        >(std::forward<CallArgs>(params)...);
    }
};

# ? Aug 12, 2009 05:36

Wuhao: Apr 22, 2002; Pimpin' Lenin

Avenging Dentist posted:

And just think, you can actually do practical stuff that looks more-or-less like that.

Yeah, you know, do we call people with ungodly C++ boost skill "gurus," or "sith lords?"

# ? Aug 12, 2009 05:38

4VAlien: Jun 19, 2008

By the time i was called in the program would sometimes not crash while trying to create an inventarisation snapshot to use on some hand scanners, so it wasnt really dead yet. A wonder that it ever worked though:

code:

Dim Decimaal As String
Dim Decimaal2 As Double

...

Decimaal = sqlDSaanmaken.Tables(0).Rows(iRow).Item(iCol)
Decimaal2 = RoundOff(Decimaal, 3)
Decimaal = (Str(Decimaal2))

...

Private Function RoundOff(ByVal value As Double, ByVal digits As Integer) As Double
        Dim shift As Double

        shift = 10 ^ digits
        RoundOff = CInt(value * shift) / shift
    End Function

When i rewrote it i could do with one button and two comboboxes instead of the 6 forms that this monstrosity was using. Because i was feeling generous i also bloated my new version with an additional progressbar

# ? Aug 12, 2009 12:42

Scaevolus: Apr 16, 2007

4VAlien posted:

code:

Decimaal
Decimaal2
Decimaal sqlDSaanmaken
Decimaal2 Decimaal
Decimaal Decimaal

Here's the real horror.

# ? Aug 12, 2009 14:33

Lurchington: Jan 2, 2003; Forums Dragoon

I had assumed that was in another language (Dutch)
http://www.allwords.com/word-decimal.html

But yeah:

code:

int int
int int2

is a horror in any language

# ? Aug 12, 2009 15:28

Mill Town: Apr 17, 2006

Scaevolus posted:

Here's the real horror.

Maybe the first programmer working on this was Dutch?

E: f;b

# ? Aug 12, 2009 15:30

deedee megadoodoo: Sep 28, 2000; Two roads diverged in a wood, and I, I took the one to Flavortown, and that has made all the difference.

Mill Town posted:

Maybe the first programmer working on this was Dutch?

E: f;b

That still doesn't forgive this...

code:

Dim Decimaal As String

# ? Aug 12, 2009 18:49

Lexical Unit: Sep 16, 2003

code:

class a_context { /* ... */ };
class b_context { /* ... */ };
class c_context { /* ... */ };

template<class Context>
class _foo
{
  // ...
};

#define a_foo _foo<a_context>
#define b_foo _foo<b_context>
#define c_foo _foo<c_context>

# ? Aug 12, 2009 18:50

Zhentar: Sep 28, 2003; Brilliant Master Genius

HatfulOfHollow posted:

That still doesn't forgive this...
code:
Dim Decimaal As String

Nor the fact that he named it RoundOff because Round is already taken by a standard VB function.

# ? Aug 12, 2009 21:29

Tinyn: Jan 10, 2003

Avenging Dentist posted:

Here's some code I wrote for a memoizer for C++0x:

code:

namespace detail
{
    template<typename T>
    struct remove_const_ref :
        std::remove_const<
            typename std::remove_reference<T>::type
        >
    {};

    template<typename T,typename U>
    struct is_similar : 
        std::is_same<
            typename detail::remove_const_ref<T>::type,
            typename detail::remove_const_ref<U>::type
        >
    {};
}

template <typename T,typename Ret,typename ...Args>
class memoizer_base<T,Ret (Args...)>
{
public:
    // ...

    template<typename ...CallArgs>
    result_reference operator() (CallArgs &&...params)
    {
        return call<
            typename boost::mpl::if_<
                typename detail::is_similar<CallArgs,Args>::type,
                CallArgs&&,
                typename detail::remove_const_ref<Args>::type&&
            >::type...
        >(std::forward<CallArgs>(params)...);
    }
};

What the christ is that doing?

# ? Aug 16, 2009 05:42

Avenging Dentist: Oct 1, 2005; oh my god is that a circular saw that does not go in my mouth aaaaagh

Tinyn posted:

What the christ is that doing?

It forwards a call from operator() to call, converting from the types passed in to the original type specified in the memoizer class template where necessary. It's there to eliminate unnecessary copying by supporting C++0x move semantics whenever possible. I have a comment explaining more-or-less that in the code itself, but where's the fun in that?

Avenging Dentist fucked around with this message at 06:05 on Aug 16, 2009

# ? Aug 16, 2009 06:03

Intel Penguin: Sep 14, 2007; hooray

Incoherence posted:

Another way to make people crazy: if you ever have an opportunity to write a 4-level nested loop, use the variables i, ii, iii, and iiii.

The real WTF is, of course, the fact that you're writing a 4-level nested loop, but that's another story.

what about i, ii, iii, and iv?

# ? Aug 16, 2009 09:19

Mill Town: Apr 17, 2006

Intel Penguin posted:

what about i, ii, iii, and iv?

A well-written program would use both IIII and IV.

# ? Aug 16, 2009 14:57

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

What's

from, QCS?

# ? Aug 16, 2009 15:04

Mill Town: Apr 17, 2006

Otto Skorzeny posted:

What's from, QCS?

E/N.

# ? Aug 16, 2009 15:25

Ensign Expendable: Nov 11, 2008; Lager beer is proof that god loves us; Pillbug

Mill Town posted:

A well-written program would use both IIII and IV.

Interchangeably.

# ? Aug 16, 2009 16:09

Myrddin Emrys: Jul 3, 2003; Ho ho ho, Pac-man!

Wow, cleaning up our old team's code was horrific.

First of all, one of our biggest, most complex products has a pretty large user database. We store TONS of information on users, that's sort of the point of the software (not going to say what it does, though, it's not important).

Eventually our WHOLE development team was fired and a new team (us) brought in. The guys that were working on this did get a chance to talk to the old team on the way out.

I guess what happened is this: All the user info was stored in one table. In fact, everything was stored in one table, the whole database (several gigs at this point, it's even larger now) was on goddamn table.

At one point, they actually managed to max out the number of columns a table can have in SQL Server 2000. Then they were required to add two more columns. So, instead of normalizing ANYTHING, they go ahead and just... delete what they considered the two least used columns to make room for the new ones. This was back before any sort of backup policy was in place, meaning that data was gone FOREVER. They also didn't tell anyone that they did it, so they had to discover this a year or so later.

Same guys also had a few favorite VB tricks, including using one variable for pretty much everything (I guess they were rationing variables back in the day or something) and determining what to do using if statements to figure out the type. Yeah, I can see an application or two of that maybe, but using ONE VARIABLE ON AN ENTIRE SITE is just insanity.

Also I had to edit a function they wrote, in classic ASP. It was one function to generate the code necessary to populate a java-applet-based graph. Not very tough at all, just feed an applet some numbers. It took them 3000+ lines of code spread amongst 3 or 4 include files. They also had not heard of a thing called concatenation, so every single minor variation of things like the select statements (which of course were all inline) had its own if statement and was written out in full. Also no input validation, so yes people could SQL inject like crazy. And yes, people DID SQL inject us. Imagine our surprise one day when one of our legacy product's production environment was suddenly missing our "users" table.

Myrddin Emrys fucked around with this message at 04:14 on Aug 17, 2009

# ? Aug 17, 2009 04:12

Zombywuf: Mar 29, 2008

I look at the source code for our site for five minutes and I find this:

code:

database.AddInParameter(spCommand, "xml", DbType.String, StringUtilities.LimitLength(flightsXml, maxXmlSize));

But then it gets worse:

code:

public static string AssignNotNullString(string text)
{
	return text == null ? "" : text;
}

public static string LimitLength(string text, int maxLength)
{
	if (maxLength < 0) return "";
	text = AssignNotNullString(text);
	if (maxLength > text.Length) maxLength = text.Length;
	return text.Substring(0, maxLength);
}

CoC challenge, find all the flaws in the above code.

# ? Aug 17, 2009 15:33

Jethro: Jun 1, 2000; I was raised on the dairy, Bitch!

The only one I see that's worth crying about is the willy-nilly truncation of the XML. I'd say that's a great way to magically create malformed XML. Maybe the fact that he's storing XML in a text field of some sort instead of an XML field, but I guess this is the "coding horrors" thread, not the "critique my dba by looking at 10 lines of application code" thread.

# ? Aug 17, 2009 16:36

Flobbster: Feb 17, 2005; "Cadet Kirk, after the way you cheated on the Kobayashi Maru test I oughta punch you in tha face!"

I like that the helper function they wrote to handle the null string case requires about twice as much typing as the expression itself.

code:

str = StringUtilities.AssignNotNullString(str);
str = str == null ? "" : str;

Hell, if you don't have to support an ancient version of the language, it's even worse:

code:

str = StringUtilities.AssignNotNullString(str);
str = str ?? "";

# ? Aug 17, 2009 16:52

Zombywuf: Mar 29, 2008

Jethro posted:

The only one I see that's worth crying about is the willy-nilly truncation of the XML. I'd say that's a great way to magically create malformed XML. Maybe the fact that he's storing XML in a text field of some sort instead of an XML field, but I guess this is the "coding horrors" thread, not the "critique my dba by looking at 10 lines of application code" thread.

You score 1 point. The other flaws are in the 2 functions posted. There are several flaws.

# ? Aug 17, 2009 17:04

tef: May 30, 2004; -> some l-system crap ->

Jethro posted:

not the "critique my dba by looking at 10 lines of application code" thread.

that was written by the lead architect*.

many of his contributions have been posted before:

code:

namespace Core.Framework.DataType.BooleanUtils
{
	public static class BooleanFormatter
	{
		/// <summary>
		/// returns string representation of boolean value based on passed definitions of <value>true</value> and <value>false</value>
		/// </summary>
		/// <param name="value"></param>
		/// <param name="trueValue"></param>
		/// <param name="falseValue"></param>
		/// <returns></returns>
		public static string Format(bool value, string trueValue, string falseValue)
		{
			return value ? trueValue : falseValue;
		}
	}
}

* although successive re-orgs may have changed his job title beyond recognition, he is still the primary author of most of the website.

edited to add: :woop:

I don't work there :toot:

# ? Aug 17, 2009 17:17

dwazegek: Feb 11, 2005; WE CAN USE THIS

Zombywuf posted:

You score 1 point. The other flaws are in the 2 functions posted. There are several flaws.

The maxLength < 0 check is stupid, just throw an argument exception. Or at least check for <= 0.

Calling SubString even if no truncation is needed.

# ? Aug 17, 2009 17:28

HFX: Nov 29, 2004

Zombywuf posted:

You score 1 point. The other flaws are in the 2 functions posted. There are several flaws.

He's setting the value of maxLength to the text length. We he then returns the substring, he will return the string the same as it went into the function.

# ? Aug 17, 2009 17:38

Zombywuf: Mar 29, 2008

dwazegek posted:

The maxLength < 0 check is stupid, just throw an argument exception. Or at least check for <= 0.

Calling SubString even if no truncation is needed.

Ding ding 2 points.

The last one I'd count is somewhat stylistic. But if you're masking null pointer errors in your data layer, urdoinitrong.

# ? Aug 17, 2009 17:45

Myrddin Emrys: Jul 3, 2003; Ho ho ho, Pac-man!

HFX posted:

He's setting the value of maxLength to the text length. We he then returns the substring, he will return the string the same as it went into the function.

That's only if maxLength exceeds text length, though. If maxLength is between 0 and text.Length it stays the same.

# ? Aug 17, 2009 17:57

HFX: Nov 29, 2004

Myrddin Emrys posted:

That's only if maxLength exceeds text length, though. If maxLength is between 0 and text.Length it stays the same.

Right which is completely retarded. Sadly this code reminds me of a lot of the code I see around my work. I'm going to have to post some when I find some.

# ? Aug 17, 2009 18:33

Myrddin Emrys: Jul 3, 2003; Ho ho ho, Pac-man!

HFX posted:

Right which is completely retarded. Sadly this code reminds me of a lot of the code I see around my work. I'm going to have to post some when I find some.

Sure it's not ideal. I just meant he wouldn't necessarily be returning the same string, which is what you originally said.

# ? Aug 17, 2009 18:54

McGlockenshire: Dec 16, 2005; GOLLOCKS!

We have a code test that we make candidates take before we consider giving them an interview. We ask them to build a little song database, give it at least three specific views of the data, and to switch between views using a modern JS library.

Nobody, not one person, has ever passed the test. Not even me. This gem came in today, in part of the submitter's home-grown application code, including a class that wraps PHP's mysql_* functions:

php:

<?
class songs_global { 
// ...
    public function fixString($Instring=NULL){
    //Use this on any text you pass into the database.
    //Queries will fail if any variable contains a single quote.
    //First replaces addslashes /' with just a single quote, 
    //then replaces single quotes with two single quotes. 
    //ie bee/'s knees or bee's knees becomes bee''s knees,
    //then uses the first quote to escape the second
    //so INSERT 'bee''s knees' is interpreted as
    //INSERT 'bee's knees'

        if($Instring == NULL){
            return NULL;
        }
        $OutString = str_replace("'", "''", str_replace("\'", "'", $Instring)); 
        return $OutString;
    }?>

Mind you, this method isn't in the database adapter. Oh, no, it's in the "global" class. And it doesn't even do what he thinks it does, it just makes a horrible backslashy mess.

The entire "global" class is a horrible, horrible thing that makes me want to shoot him. Thankfully I don't have a gun, I'll just get to ask him what the gently caress he's thinking if the boss decides he's worth interviewing.

McGlockenshire fucked around with this message at 23:50 on Aug 18, 2009

# ? Aug 18, 2009 23:41

Patashu: Jan 7, 2009

Why would it make a backslashy mess? Unless it replaces the second instance with the first, as opposed to the first instance with the second

# ? Aug 19, 2009 00:08

McGlockenshire: Dec 16, 2005; GOLLOCKS!

On his own demo for his own code, submitting:

"Testy'test"test"

transforms into

\"Testy\'test\"test\"

I have no idea what's even going on.

I think he might have magic_quotes_gpc enabled. (Non-PHP people: magic_quotes_gpc is a misfeature that automatically tries to escape quotes in all input.)

# ? Aug 19, 2009 00:48

ohgodwhat: Aug 6, 2005

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?

# ? Aug 19, 2009 05:18

Kelson: Jan 23, 2005

McGlockenshire posted:

...
php:
<?
...
        $OutString = str_replace("'", "''", str_replace("\'", "'", $Instring)); 
        return $OutString;
    }?>
Mind you, this method isn't in the database adapter. Oh, no, it's in the "global" class. And it doesn't even do what he thinks it does, it just makes a horrible backslashy mess.

The entire "global" class is a horrible, horrible thing that makes me want to shoot him. Thankfully I don't have a gun, I'll just get to ask him what the gently caress he's thinking if the boss decides he's worth interviewing.

I like how it lets "don\\'t escape my injection" through

# ? Aug 19, 2009 05:27

Zakalwe

May 12, 2002

Wanted For:

Terrorism
Kidnapping
Poor Taste
Unlawful Carnal Gopher Knowledge

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.

# ? Aug 19, 2009 18:47

Lexical Unit: Sep 16, 2003

code:

void pants_event(char* data, size_t size, long id)
{
	switch (id)
	{
		case 1:
			handle_pants (data, size, id);
			break;
		case 2:
			handle_pants (data, size, id);
			break;
		case 3:
			handle_pants (data, size, id);
			break;
		case 4:
			handle_pants (data, size, id);
			break;
	}
}

void handle_pants(char* data, size_t size, long id)
{
	switch (id)
	{
		case 1:
			{
				// do stuff
			}
			break;
		case 2:
			{
				// do stuff
			}
			break;
		case 3:
			{
				// do stuff
			}
			break;
		case 4:
			{
				// do stuff
			}
			break;
	}
}

# ? Aug 19, 2009 20:55

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

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 19:56

Lexical Unit: Sep 16, 2003

Oh the things I could draw if posts were whiteboards...

# ? Aug 19, 2009 22:36

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!

«‹›1503 »