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!

«‹›1501 »

defmacro: Sep 27, 2005; cacio e ping pong

Mikey-San posted:

meh

Thanks

defmacro fucked around with this message at 15:42 on Apr 8, 2008

# ? Apr 8, 2008 15:40

Adbot: ADBOT LOVES YOU

# ? Apr 24, 2024 09:01

brae: Feb 2, 2006

nebby posted:

For what it's worth, the "IsNullOrEmpty" phrase is pretty cumbersome, I much prefer Ruby's "blank?" which implies it is either null or empty (this applies to both arrays and strings.)

Just a note, blank? is an extension provided by one of the Active* libraries that rails uses. There isn't a blank? in vanilla ruby. For strings and standard containers you can use nil? and empty? in conjunction. I think this is what blank? does behind the scenes, anyway.

# ? Apr 8, 2008 19:07

dustgun: Jun 20, 2004; And then the doorbell would ring and the next santa would come

brae posted:

Just a note, blank? is an extension provided by one of the Active* libraries that rails uses. There isn't a blank? in vanilla ruby. For strings and standard containers you can use nil? and empty? in conjunction. I think this is what blank? does behind the scenes, anyway.

http://dev.rubyonrails.org/browser/trunk/activesupport/lib/active_support/core_ext/blank.rb
It's simple enough to just toss into whatever project you need to use it in.

# ? Apr 8, 2008 20:22

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

dustgun posted:

http://dev.rubyonrails.org/browser/trunk/activesupport/lib/active_support/core_ext/blank.rb
It's simple enough to just toss into whatever project you need to use it in.

Hell, just including all of ActiveSupport is a pretty decent idea. There's a lot of good stuff in there.

# ? Apr 8, 2008 21:02

volkadav: Jan 1, 2008; Guillotine / Gulag 2020

code:

public static class ThingSqlHelper {
    private static final String sql = "select * from things where foo='bar'";

    public static String getSQL() {
        StringBuffer sb = new StringBuffer(this.sql);
        return sb.toString();
    }
}

(from memory, I don't have the code in front of me at the moment)

The same system includes an ACL implementation based on hidden text in table cells that is read/written with javascript and big honking dynamically written javascript arrays containing all the users and their group memberships and :psyduck:

# ? Apr 9, 2008 03:08

Mikey-San: Nov 3, 2005; I'm Edith Head!

GT_Onizuka posted:

Thanks

No no, it wasn't in response to anyone. I posted something, realized I spoke too quickly and the post was bad, and removed it. That's all. I would've just deleted the post if I could've.

# ? Apr 9, 2008 06:48

defmacro: Sep 27, 2005; cacio e ping pong

Mikey-San posted:

No no, it wasn't in response to anyone. I posted something, realized I spoke too quickly and the post was bad, and removed it. That's all. I would've just deleted the post if I could've.

No problem, I just like being mean

.

To contribute, I recently found this and almost spit out my drink laughing.

code:

                                // QUERY EXECUTION
// the FROM field starts with ", "...take it out
//if (varsToTable.size() > 0) 

//  BBB  U  U  GGG
//  B  B U  U G
//  BBB  U  U G GG
//  B  B U  U G  G
//  BBB   UU   GG
//
// more code

# ? Apr 9, 2008 13:27

bt_escm: Jan 10, 2001

The best/worst one I've seen is

php:

<?
if(false){


50+ lines of code that aren't used any more

}
?>

I guess the developer didn't need those lines of code anymore and didn't care or understand what php was going to do with them everytime the script loaded.

# ? Apr 9, 2008 17:43

more falafel please: Feb 26, 2005; forums poster

bt_escm posted:

The best/worst one I've seen is
php:
<?
if(false){


50+ lines of code that aren't used any more

}
?>
I guess the developer didn't need those lines of code anymore and didn't care or understand what php was going to do with them everytime the script loaded.

In the project I just finished, there's thousands of lines that are either #if 0'd out or #ifdef JOHNSMITH, where John Smith is a programmer who hasn't worked here in 2 years.

# ? Apr 9, 2008 21:12

Triple Tech: Jul 28, 2006; So what, are you quitting to join Homo Explosion?

It pisses me off that people don't audit their poo poo and stuff like this comes up.

90% of all the bugs I've had to deal with regarding inherited/maintained code is lack of auditing.

Code that makes me cry is code without an audit history/trail.

# ? Apr 9, 2008 22:12

rotor: Jun 11, 2001; classic case of pineapple derangement syndrome

Triple Tech posted:

Code that makes me cry is code without an audit history/trail.

I dunno man, I'd start buying kleenex in bulk if I were you.

# ? Apr 9, 2008 23:13

npe: Oct 15, 2004

poo poo, if I can get a few people to sleepily stare at my code for a few seconds during a code review, that's a good day.

# ? Apr 9, 2008 23:40

_aaron: Jul 24, 2007; The underscore is silent.

yaoi prophet posted:

poo poo, if I can get a few people to sleepily stare at my code for a few seconds during a code review, that's a good day.

I must be really lucky; code reviews at my office tend to be very useful. While we haven't recently had time to make sure that every single line is optimized for performance or anything like that, people always come to the meetings with suggestions and ideas.

# ? Apr 10, 2008 03:39

Jewish Bear: Jul 28, 2007

Randomosity posted:

Holy poo poo.

I've seen some awful code on my job... but THIS has to be the worst thing I've ever seen.

The function is used to create a series of dropdown boxes to select month, day, year, hour, and minute.

php:

<?
function displayHTMLMenu( $type, $name, $class, $value_digits, $value_selected, $value_top_name, $value_top_value, $on_change_function_name, $return_or_echo_content = "echo" )
{
    //global $CM_app_system_path;

    //    get menu data START
    if ($type == "month_names")
    {
        if( $_SESSION["sessionStat"]["lowresolution"] )
        {
            $nxtFile = LIST_DATA_FILES_PATH . "list_date_month_names_abbrv.txt";
        }
        else
        {
            $nxtFile = LIST_DATA_FILES_PATH . "list_date_month_names.txt";
        }
    }
    else if ($type == "day_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_day_numbers.txt";
    }
    else if ($type == "year_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_year_numbers.txt";
    }
    else if ($type == "year_numbers_current_and_previous_in_january")
    {
        // no file for year_numbers_current_and_previous_in_january, generate dynamically
        $all_data_array[] = array (date("Y"), date("Y"), date("Y"));
        
        if (date("M") == "Jan")
        {
            $all_data_array[] = array (date("Y")-1, date("Y")-1, date("Y")-1);
        }
    }
    else if ($type == "hour_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_hour_numbers.txt";
    }
    else if ($type == "minute_numbers")
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_minute_numbers.txt";
    }
    else    // default
    {
        $nxtFile = LIST_DATA_FILES_PATH . "list_date_minute_numbers.txt";
    }

    if( isset( $nxtFile ) && file_exists( $nxtFile ) )
    {
        $fd = fopen ($nxtFile, "r") or die ("Could not open file");
        flock ($fd, 2);
        while (!feof($fd))
        {
            $buffer = fgets ($fd, 4096);
            $next_line_array = split("\|", $buffer);
            $all_data_array[] = $next_line_array;
            //echo $all_data_array[0];
        }
        flock ($fd, 3);
        fclose($fd);
    }
    else
    {
        // file not found
        //echo "<h1>file not found</h1>";
    }
    //    get menu data END

    $content_to_return = "<select name=\"$name\" class=\"$class\" onchange=\"$on_change_function_name(this);\">";
    
    if (!empty ($value_top_name) || !empty ($value_top_value))
    {
        $content_to_return .= "<option value=\"$value_top_value\">$value_top_name";
    }

    foreach ($all_data_array as $next_data_array)
    {
        //echo $next_data_array[0];
        if ($next_data_array[0] != "")
        {
            //    get next value START
            if ($value_digits == 1)    // no leading zeros
            {
                $next_value = $next_data_array[1];
            }
            else if ($value_digits == 2)    // leading zeros
            {
                $next_value = $next_data_array[2];
            }
            else
            {
                $next_value = $next_data_array[2];    // leading zeros, default
            }
            //    get next value END

            if ($next_value == $value_selected)
            {
                $content_to_return .= "<option value=\"$next_value\" selected>$next_data_array[0]";
            }
            else
            {
                $content_to_return .= "<option value=\"$next_value\">$next_data_array[0]";
            }
        }
    }

    $content_to_return .= "</select>";
    
    if( $return_or_echo_content == "return" )
    {
        return $content_to_return;
    }
    else
    {
        echo $content_to_return;
    }
?>

If that was me doing that I would stab myself in the face. No joke. Why would anyone even think of doing that instead of using a for loop?

# ? Apr 10, 2008 08:45

JingleBells: Jan 7, 2007; Oh what fun it is to see the Harriers win away!

I found this gem on a project we inherited:

code:

public string getProperty(string key) 
{
    IDictionaryEnumerator enumer = map.GetEnumerator();

    while(enumer.MoveNext()) 
    {
        if (key.Equals(enumer.Key)) 
        {
            return (string)enumer.Value;
        }
    }

    //return null if the value doesn't exist
    return null;
}

Which was replaced with:

code:

public string getProperty(string key) 
{			
    return ((map[key]==null)?null:map[key].ToString());
}

# ? Apr 10, 2008 11:32

zootm: Aug 8, 2006; We used to be better friends.

I think either some spacing or avoiding the ternary operation altogether would make the "after" code in that example a lot easier to read, JingleBells.

I realise that readability wasn't the "horror" in the original, though.

# ? Apr 10, 2008 14:20

TSDK: Nov 24, 2003; I got a wooden uploading this one

zootm posted:

I think either some spacing or avoiding the ternary operation altogether would make the "after" code in that example a lot easier to read, JingleBells.

It would also help in terms of performance, because then you're not doing two lookups:

code:

public string getProperty(string key) 
{
    map::iterator property = map[key]; // (I don't know the synax)
    return ( property == null ? null : property.ToString() );
}

More readable code == faster code. Who'd have thunk it?

# ? Apr 10, 2008 14:32

JingleBells: Jan 7, 2007; Oh what fun it is to see the Harriers win away!

TSDK posted:

It would also help in terms of performance, because then you're not doing two lookups:
code:
public string getProperty(string key) 
{
    map::iterator property = map[key]; // (I don't know the synax)
    return ( property == null ? null : property.ToString() );
}
More readable code == faster code. Who'd have thunk it?

It's C#, and you're probably right about performance. As for readability yes, that's generally not one of my fortes

# ? Apr 10, 2008 14:43

FrantzX: Jan 28, 2007

code:

public String GetProperty(String key)
{
     ValueType value;
     if(map.TryGetValue(key, out value) == true) return value.ToString();

     return null;
}

I'd do it this way for only one lookup plus it won't throw an exception is the key is not found in the dictionary.

# ? Apr 10, 2008 21:31

floWenoL: Oct 23, 2002

FrantzX posted:

code:
public String GetProperty(String key)
{
     ValueType value;
     if(map.TryGetValue(key, out value) == true) return value.ToString();

     return null;
}
I'd do it this way for only one lookup plus it won't throw an exception is the key is not found in the dictionary.

Gotta love that "== true".

# ? Apr 10, 2008 22:17

Victor: Jun 18, 2004

Gotta protect against FILE_NOT_FOUND!

# ? Apr 10, 2008 22:53

dustgun: Jun 20, 2004; And then the doorbell would ring and the next santa would come

Victor posted:

Gotta protect against FILE_NOT_FOUND!

Bool.FileNotFound :colbert:

# ? Apr 10, 2008 23:23

Lexical Unit: Sep 16, 2003

So we use middleware to pass serialized objects around a network of machines. All code is c++. We have to interface with other people's code over this middleware. They define the objects that get sent back and forth over the middleware. Here's a "snippet" (not really, but to give you an idea of the horror) of their headers:

code:

#define int_32 long
#define int_64 long long

typedef struct {
	int_32 a;
	char b[512];
	double c;
	int_64 d;
	float f;
} foo;

This code has to be complied on a mix of 64 and 32 bit machines and there's no way to tell if a message any given machine gets was sent from a 32 bit machine or a 64 bit machine. Also, little vs big endianness comes into play as the endianness of the serialized object doesn't necessarily have to match the endianness of either the sending or receiving machines.

...

:argh:

# ? Apr 10, 2008 23:58

Surge Strip: Nov 2, 2005; Takin' hits

Another one I found today. Apparently the programmer did not know that the TListBox in C++ Builder has an ItemIndex property, which, oddly enough, will tell you what item is selected!

code:

for (cnt = 0; cnt < ListBox->Count; cnt++)
   {
   if (ListBox->Selected[cnt])
      {
      /*stuff with cnt*/
      break;
      }
   }

Also, classes that abuse OOP principles. Next to no private members, and worst of all, all the data is in public pointers, that have no documentation.

# ? Apr 11, 2008 05:17

Standish: May 21, 2001

Most pointless overuse of "enterprise" technology I've ever seen, a CORBA interface that consists of exactly one function that takes exactly one inout parameter (an XML string) and returns nothing:

code:

//the only declaration in the IDL file:

void executeTransaction(inout string xmlDocument)
       raises(Isis::InterfaceError);

And encoded in the XML param were various verbs such as authorize, query subscriber, submit request:

code:

<?xml version="1.0" encoding="UTF-8"?>
<AAA>
   <Request type=�QryAccConfAck�>
           <CoId>01</CoId>
           <CoKey>123456789</CoKey>
           <AuthReqRef>88888</AuthReqRef>
           <AuthRef>1234</ AuthRef>
   </Request>
</AAA>

(This wasn't some stupid little internal project either, this was a public SMS-submission interface belonging to a major mobile network. But hey, CORBA is enterprise technology, and XML is enterprise technology, so XML-over-CORBA must be twice as enterprisey, right?)

Standish fucked around with this message at 14:44 on Apr 11, 2008

# ? Apr 11, 2008 14:41

MrMoo: Sep 14, 2000

Lexical Unit posted:

So we use middleware to pass serialized objects around a network of machines. All code is c++. We have to interface with other people's code over this middleware.

Not really middleware then, just a communications system. Most normal message orientated middleware systems use a platform agnostic messaging format, like XDR name, value pairs, or modern behemoths like XML.

# ? Apr 11, 2008 14:45

Lexical Unit: Sep 16, 2003

Ok, but, the software is called "middleware." And, conceptually, it stands between two computers. Also, the messaging is platform agnostic. What you put in is what you get out, no matter what platform you're on. So if a 64 bit machine puts in a 20 bit object, a 32 bit machine will receive a 20 bit object, even if the source for that object compiles to a 16 bit object under 32 bit compilation. Hence the horror.

# ? Apr 11, 2008 14:54

MrMoo: Sep 14, 2000

Lexical Unit posted:

Hence the horror.

I see a float too, float's even though IEEE specified are not standard across platforms. AIX/PowerPC has great issues with this.

Technically that middleware is tied to the sending system. TIBCO Rendezvous works across multiple platforms and has an agnostic message format, together with broken IEEE floating points. I'm amazed at the number of clients who expect it to translate even character encoding automagically between hosts.

MrMoo fucked around with this message at 16:24 on Apr 11, 2008

# ? Apr 11, 2008 16:08

fansipans: Nov 20, 2005; Internets. Serious Business.

Lexical Unit posted:

Also, little vs big endianness comes into play as the endianness of the serialized object doesn't necessarily have to match the endianness of either the sending or receiving machines.

...

Whenever I have to deal with endian issues I leave a trail of tears!

...

One time I was freaking out because I saw a little endian - my friend said don't worry - it must have been the LSB!

...

Taking account of endianness always fills me with reservations.

...

~~Data having network byte order is always lazy, drunk, and poor!~~

...

Thanks! I'll be here all week! Please remember to tip your waitresses!

# ? Apr 11, 2008 16:14

rotor: Jun 11, 2001; classic case of pineapple derangement syndrome

the worst thing about endianness for me is that an old coworker insisted on calling it byte sex every opportunity he got, which drove me up the wall.

# ? Apr 11, 2008 16:18

Lexical Unit: Sep 16, 2003

fansipans posted:

...
...
...
...

...

Are you making fun of me? It was a pause for dramatic effect! :smith:

MrMoo posted:

Technically that middleware is tied to the sending system.

Ah I see what you mean now. And yes, that would be an accurate assessment.

# ? Apr 11, 2008 17:07

Scaevolus: Apr 16, 2007

Project Euler Problem 22 posted:

Using names.txt (right click and 'Save Link/Target As...'), a 46K text file containing over five-thousand first names, begin by sorting it into alphabetical order

names.txt posted:

"MARY","PATRICIA","LINDA","BARBARA","MARIA","SUSAN","MARGARET","DOROTHY","LISA","NANCY",..."ALONSO"

The first part of someone's solution:

code:

#First load the file and sort it.
x = eval( '[' + open( '.../names.txt' ).readlines()[ 0 ] + ']' )
x.sort()

On principle, using eval is always wrong.

# ? Apr 12, 2008 04:49

noonches: Dec 5, 2005; It represents my lifestyle and stratus as a street savy irreverent youth, who lives large, and yet hungers for the next level in life.

Scaevolus posted:

On principle, using eval is always wrong.

In practice, it usually is too.

# ? Apr 12, 2008 04:56

JoeNotCharles: Mar 3, 2005; Yet beyond each tree there are only more trees.

The code I'm working on right now is mostly okay, but I just found functions called "getOnlyFileOrDirectoryNameNoPath" and "getOnlyPathNoFileOrDirectoryName".

# ? Apr 12, 2008 06:32

Lankiveil: Feb 23, 2001; Forums Minimalist

JoeNotCharles posted:

The code I'm working on right now is mostly okay, but I just found functions called "getOnlyFileOrDirectoryNameNoPath" and "getOnlyPathNoFileOrDirectoryName".

So long as there's nothing like this:

code:

fopen(getOnlyPathNoFileOrDirectoryName(x) . getOnlyFileOrDirectoryNameNoPath(x));

Right?

# ? Apr 12, 2008 09:09

bmoyles: Feb 15, 2002; United Neckbeard Foundation of America; Fun Shoe

my two favorite coding horrors from this week

code:

while(list($key,$val) = each($_POST)) {
        $$key = $val;
}


while(list($key,$val) = each($_GET)) {
        $$key = $val;
}

and

code:

if($accountadmin == 'Y') {
       echo "<p><a
href=\"listaccounts.php?username=$username&password=$password\">List Accounts</a><p>";
       echo "<p><a
href=\"addaccount.php?username=$username&password=$password\">Add
Account</a><p>";
}
?>

That last hunk there is awesome as once authenticated, it passes the username AND password around in the query string FOR EVERY REQUEST but pages that require authentication DONT ACTUALLY CHECK TO SEE IF YOU'RE AUTHENTICATED!!! :psypop:

The only thing the username is ever used for is to look up data in the database
:wtc:

# ? Apr 12, 2008 15:54

JoeNotCharles: Mar 3, 2005; Yet beyond each tree there are only more trees.

Lankiveil posted:

So long as there's nothing like this:
code:
fopen(getOnlyPathNoFileOrDirectoryName(x) . getOnlyFileOrDirectoryNameNoPath(x));
Right?

Um, well, more like:

code:

obj.pathName = getOnlyPathNoFileOrDirectoryName(x)
obj.fileName = getOnlyFileOrDirectoryNameNoPath(x)

<much, much later>

fopen(obj.pathName + obj.fileName)

There's actually a reason, though, which is that the fileName alone is also used in the meantime. Still, it would make way more sense to store the filename (for display purposes) and the whole path (for access), instead of the filename and just the path.

Oh, also, this project's most common coding style puts spaces around everything:

code:

void SomeObject :: SomeMethod ( int arg1, unsigned int arg2 )
{
  if ( ( ( condition1 && condition2 ) && ! ( condition3 && condition4 ) || ...

I prefer "func(args)", but I do have some sympathy for "func( args )" people. I've never before in my life seen "func (args)", but "func ( args )" is just ridiculous.

# ? Apr 12, 2008 17:48

pseudopresence: Mar 3, 2005; I want to get online...
I need a computer!

I tend to use func( args ) and keyword ( args ), but void Class :: Method ( args ) is just silly.

# ? Apr 12, 2008 19:18

_aaron: Jul 24, 2007; The underscore is silent.

No space before the comma? Clearly this person doesn't know how to properly use white space.

# ? Apr 12, 2008 19:47

Adbot: ADBOT LOVES YOU

# ? Apr 24, 2024 09:01

Factor Mystic: Mar 20, 2006; Baby's First Post-Apocalyptic Fiction

You should suggest more vertical whitespace. An extra line break for every line or it's not readable :colbert:

# ? Apr 12, 2008 20:18

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!

«‹›1501 »