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 »

Zemyla: Aug 6, 2008; I'll take her off your hands. Pleasure doing business with you!

bouyancy posted:

I stumbled across this at work a while back. Somebody decided to be clever and make a data structure that could read any value from a bunch of different tables and queries. The setRow method is called on each row returned from a query. And they wonder why the appliction runs slowly.
code:
public class Row {

    public static int USERNAME = 0;
    public static int USER_PHONE = 1;
    public static int USER_DOB = 2;
    ...


    public void setRow(ResultSet result) {
        try { values[Row.USERNAME] = result.getString("username")} catch (Exception ignored) {}
        try { values[Row.USER_PHONE] = result.getString("user_phone")} catch (Exception ignored) {}
        try { values[Row.USER_DOB] = result.getString("user_dob")} catch (Exception ignored) {}
        
	... 

        continues like this for about 300 lines
    }

}

So if the result set is invalid, it throws and catches three hundred exceptions?

The punishment should fit the crime. Throw three hundred baseballs at him, and then tell him that's how the computer feels after running that code.

# ? Mar 31, 2009 11:51

Adbot: ADBOT LOVES YOU

# ? May 13, 2024 23:47

bouyancy: Apr 17, 2003

Zemyla posted:

So if the result set is invalid, it throws and catches three hundred exceptions?

The punishment should fit the crime. Throw three hundred baseballs at him, and then tell him that's how the computer feels after running that code.

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.

# ? Mar 31, 2009 16:36

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

geetee: Feb 2, 2004; >;[

BigRedDot posted:

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

I really shouldn't have...

# ? Apr 1, 2009 18:42

mr_jim: Oct 30, 2006; OUT OF THE DARK

A friend just told me about a guy he works with who prefaces every function name with his user name, as in:

code:

userName_funcName();

# ? Apr 2, 2009 20:29

POKEMAN SAM: Jul 8, 2004

mr_jim posted:

A friend just told me about a guy he works with who prefaces every function name with his user name, as in:
code:
userName_funcName();

code ownership baby

# ? Apr 2, 2009 20:58

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

my function is more optimized than your function, good thing we namespaced them, use mine

# ? Apr 2, 2009 21:55

Spell: Nov 20, 2000; Eat your vegetables or you'll DIE!

That's great when you want to call a function you have to remember who wrote it. Then you know who to blame when it fails.

# ? Apr 2, 2009 23:17

blorpy: Jan 5, 2005

mr_jim posted:

A friend just told me about a guy he works with who prefaces every function name with his user name, as in:
code:
userName_funcName();

maybe he just saw all those examples using myFunc and decided to personalize it a bit further :haw:

# ? Apr 2, 2009 23:17

mr_jim: Oct 30, 2006; OUT OF THE DARK

Spell posted:

That's great when you want to call a function you have to remember who wrote it. Then you know who to blame when it fails.

I told my friend that it was so he'd know who to punch in the face for not knowing about the "blame" feature of most SCMs.

# ? Apr 2, 2009 23:29

Wheany: Mar 17, 2006; Spinya^{ha^{ha^haha}ha}ha_{ha_{ha_haha}ha}ha!; Doctor Rope

code:

% grep -RP "\Wsome_flag\W" *
hal/driver.c:extern int some_flag;
hal/driver.c:if(some_flag == FALSE)
hal/driver.c:if(some_flag == FALSE) 
device.c:int some_flag = FALSE;
device.c:some_flag = FALSE;
device.c:some_flag = FALSE;
device.c:some_flag = TRUE;

Those two ifs should test for TRUE and FALSE, so there's a bug.

But also:
The flag is defined and manipulated, but never tested, in device.c.
It is declared extern and tested, but never manipulated, in driver.c.

# ? Apr 8, 2009 11:03

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

Wheany posted:

But also:
The flag is defined and manipulated, but never tested, in device.c.
It is declared extern and tested, but never manipulated, in driver.c.

Why would you think this is a coding horror? Sure, it's a bit of an old-school way to get a status from a module, but then you are working in C.

# ? Apr 8, 2009 11:13

Wheany: Mar 17, 2006; Spinya^{ha^{ha^haha}ha}ha_{ha_{ha_haha}ha}ha!; Doctor Rope

TSDK posted:

Why would you think this is a coding horror? Sure, it's a bit of an old-school way to get a status from a module, but then you are working in C.

Well, it doesn't really show from those lines, but there is no need for the flag to even exist in device.c. It pretty much goes like this:

code:

/* device.c */
    /* ... */
    some_flag = TRUE;
    make_device_status_true();
    /* ... */

/* driver.c */
/* ... */
make_device_status_true()
{
    if(some_flag == FALSE)
    {
        foo_device_set_status(TRUE);
    }
}
/* ... */

# ? Apr 8, 2009 11:41

ih8ualot: May 20, 2004; I like turkey and ham sandwiches

I'm in an AI group, writing an agent that can solve Minesweeper in Python.

code:

rowcount = 0
colcount = 0
while (rowcount < board.rows):
    colcount = 0
    while (colcount < board.cols):

        ...

        colcount = colcount + 1

    rowcount = rowcount + 1

I mean, I guess that's a way to do it...

ih8ualot fucked around with this message at 18:45 on Apr 8, 2009

# ? Apr 8, 2009 15:10

Zhentar: Sep 28, 2003; Brilliant Master Genius

Wheany posted:

Well, it doesn't really show from those lines, but there is no need for the flag to even exist in device.c.

That's still not much of a horror; it just looks like useless cruft that's accumulated over many revisions.

To contribute, I just came across some code I wrote 6 months ago....

code:

  s curData=$$get(recId,700,date)
  s %=$p(curData,c10,1),$p(%,c9,1)=itm,itm=itm-$p(%,c9,2)+$p(%,c9,3),$p(curData,c10,1)=%
  s %=$p(curData,c10,2),$p(%,c9,1)=amt,amt=$$addAmount($$subtractAmount(amt,$p(%,c9,2)),$p(%,c9,3)),$p(curData,c10,2)=%
  s %=$p(curData,c10,3),$p(%,c9,1)=grp,grp=grp-$p(%,c9,2)+$p(%,c9,3),$p(curData,c10,3)=%
  s %=$$put(recId,700,date,curData)
  f  s tab=$o(itm(tab)) q:tab=""  d
  . s curData=$$get(recId,700,date,tab)
  . s %=$p(curData,c10,1),$p(%,c9,1)=itm(tab),itm(tab)=itm(tab)-$p(%,c9,2)+$p(%,c9,3),$p(curData,c10,1)=%
  . s %=$p(curData,c10,2),$p(%,c9,1)=amt(tab),amt(tab)=$$addAmount($$subtractAmount(amt(tab),$p(%,c9,2)),$p(%,c9,3)),$p(curData,c10,2)=%
  . s %=$p(curData,c10,3),$p(%,c9,1)=grp(tab),grp(tab)=grp(tab)-$p(%,c9,2)+$p(%,c9,3),$p(curData,c10,3)=%
  . s %=$$put(recId,700,date,tab,curData)
  q

# ? Apr 8, 2009 19:53

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

lol you work at Epic Systems. :pwn:

# ? Apr 8, 2009 20:18

dagard: Mar 31, 2005

Todays' WTF moment, looking at some PHP login validation code written by a now long-gone coworker:

code:

setcookie ( "user", $_POST[user] );
setcookie ( "pass", $_POST[pass] );
$db = open_db( $db_login[host], $db_login[user], $db_login[pwd], $db_login[table] );
$result = db_query("SELECT * FROM perms where user = '$_POST[user]'", $db);
if (mysql_num_rows($result) != '0') {
 setcookie ( "perms", mysql_result($result, 0, groups));
 header("Location: [url]http://server/app/main.php[/url]");
}

# ? Apr 8, 2009 20:28

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

Haha, I'm not sure what the bigger WTF is, the fact that you store the user/pass unencrypted in the cookie (thus sending it over the wire every time you request a page) or the blatant SQL injection.

# ? Apr 8, 2009 20:38

dagard: Mar 31, 2005

Ryouga Inverse posted:

Haha, I'm not sure what the bigger WTF is, the fact that you store the user/pass unencrypted in the cookie (thus sending it over the wire every time you request a page) or the blatant SQL injection.

Or that we don't even CHECK the password, don't forget that.

# ? Apr 8, 2009 20:40

ExileStrife: Sep 12, 2004; Happy birthday to you!
Happy birthday to you!

dagard posted:

Or that we don't even CHECK the password, don't forget that.

THAT, and also that your 'permissions' are stored in a client side cookie.

# ? Apr 8, 2009 22:35

TOO SCSI FOR MY CAT: Oct 12, 2008; this is what happens when you take UI design away from engineers and give it to a bunch of hipster art student "designers"

dagard posted:

Or that we don't even CHECK the password, don't forget that.

If the password is incorrect, won't it fail to connect to the database? It looks like the open_db() procedure uses the auth parameters in POST to connect :psyduck:

# ? Apr 8, 2009 23:19

geetee: Feb 2, 2004; >;[

Janin posted:

If the password is incorrect, won't it fail to connect to the database? It looks like the open_db() procedure uses the auth parameters in POST to connect

Nah.

# ? Apr 8, 2009 23:25

raminasi: Jan 25, 2005; a last drink with no ice

I keep finding this idiom lying around:

code:

LocalPlace *plp = NULL;
try {
	plp = new LocalPlace;
}
catch (...) {
	plp = NULL;	//lint !e423 Creation of memory leak in assignment
}

LocalPlace::LocalPlace(void) is trivial. Naturally, plp will be compared to NULL later on down the line to see if it allocated successfully.

# ? Apr 9, 2009 02:12

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

GrumpyDoctor posted:

I keep finding this idiom lying around:
code:
LocalPlace *plp = NULL;
try {
	plp = new LocalPlace;
}
catch (...) {
	plp = NULL;	//lint !e423 Creation of memory leak in assignment
}
LocalPlace::LocalPlace(void) is trivial. Naturally, plp will be compared to NULL later on down the line to see if it allocated successfully.

If it weren't for the fact that you're setting to NULL twice, this is completely acceptable and is in fact a recommended thing to do. (Though putting the error handling in the catch statement would be even better.)

# ? Apr 9, 2009 02:17

Beardless Woman: May 5, 2004; M for Mysterious

code:

if ($line =~ /#/ || length($line) < 2) { #we also ignore any lines that are not right
    #Just a holder place so i don't have to do reverse logic
}
else {
    #print "$line\n";	 #This works right
    push (@servers, $line);		
}

code:

my @logfile = <LOGREAD>;
foreach my $logline ( @logfile) {
   chomp($logline);
   print SENDMAIL "$logline\n ";
}

And this guy's code is actually in production.

Beardless Woman fucked around with this message at 07:27 on Apr 9, 2009

# ? Apr 9, 2009 07:20

pokeyman: Nov 26, 2006; That elephant ate my entire platoon.

Beardless Woman posted:

code:

    #Just a holder place so i don't have to do reverse logic

Ah yes, that ancient coding blasphemy. Death to the not operator! All hail empty code blocks! Allah ackbar!

# ? Apr 9, 2009 16:09

Funking Giblet: Jun 28, 2004; Jiglightful!

code:

//Only Centre map if values set..
if (Latitude > 0.0 && Longitude > 0.0)
{
    CenterGoogleMap();
}

"I can't understand why it won't centre my based on the new coord's I'm giving it"

Simple, but effective.

# ? Apr 9, 2009 16:33

Lexical Unit: Sep 16, 2003

Avenging Dentist posted:

If it weren't for the fact that you're setting to NULL twice, this is completely acceptable and is in fact a recommended thing to do. (Though putting the error handling in the catch statement would be even better.)

Wouldn't LocalPlace* plp = new (std::nothrow) LocalPlace; be recommended over that code though? Seeing as all they seem to want to do is to set the pointer to NULL if new fails... Also, couldn't the catch (...) be masking any exceptions that LocalPlace's constructor might throw?

# ? Apr 9, 2009 17:14

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

pokeyman posted:

Ah yes, that ancient coding blasphemy. Death to the not operator! All hail empty code blocks! Allah ackbar!

He's using Perl (or Ruby) too, so he doesn't even have to use the not operator - unless will work just as well.

# ? Apr 9, 2009 18:28

dancavallaro: Sep 10, 2006; My title sucks

pokeyman posted:

Ah yes, that ancient coding blasphemy. Death to the not operator! All hail empty code blocks! Allah ackbar!

Apparently "holder place" is the new "placeholder".

# ? Apr 9, 2009 20:13

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

Lexical Unit posted:

Wouldn't LocalPlace* plp = new (std::nothrow) LocalPlace; be recommended over that code though? Seeing as all they seem to want to do is to set the pointer to NULL if new fails... Also, couldn't the catch (...) be masking any exceptions that LocalPlace's constructor might throw?

You can do it however you want. It's not a coding horror to check for memory allocation failure. Worst-case scenario, it's slightly more verbose than necessary.

# ? Apr 9, 2009 21:12

Kidane: Dec 15, 2004; DANGER TO MANIFOLD

Beardless Woman posted:

code:

if ($line =~ /#/ || length($line) < 2) { #we also ignore any lines that are not right
    #Just a holder place so i don't have to do reverse logic
}
else {
    #print "$line\n";	 #This works right
    push (@servers, $line);		
}

code:

my @logfile = <LOGREAD>;
foreach my $logline ( @logfile) {
   chomp($logline);
   print SENDMAIL "$logline\n ";
}

And this guy's code is actually in production.

Hahahaha, I hadn't seen the first one, that's awesome. What the hell is reverse logic? Using !~ instead?

# ? Apr 9, 2009 22:23

POKEMAN SAM: Jul 8, 2004

Kidane posted:

Hahahaha, I hadn't seen the first one, that's awesome. What the hell is reverse logic? Using !~ instead?

In C I'd wrap the whole thing in () and put !(...)

# ? Apr 9, 2009 23:56

Roseo: Jun 1, 2000; Forum Veteran

Kidane posted:

Hahahaha, I hadn't seen the first one, that's awesome. What the hell is reverse logic? Using !~ instead?

Yeah, or unless, as Ryouga said:

code:

unless ($line =~ /#/ || length($line) < 2) { #we also ignore any lines that are not right
    #print "$line\n";	 #This works right
    push (@servers, $line);		
}

# ? Apr 10, 2009 00:21

wrok: Mar 24, 2006; by angerbotSD

code:

index = 0
for row in rows:
    whatever = rows[index]
    index += 1

# ? Apr 10, 2009 15:45

Janitor Prime: Jan 22, 2004; PC LOAD LETTER

What da fuck does that mean; Fun Shoe

wrok posted:

code:

index = 0
for row in rows:
    whatever = rows[index]
    index += 1

LOL, where the hell did you see that?

# ? Apr 10, 2009 21:00

TOO SCSI FOR MY CAT: Oct 12, 2008; this is what happens when you take UI design away from engineers and give it to a bunch of hipster art student "designers"

MEAT TREAT posted:

LOL, where the hell did you see that?

It's pretty common in Python code written by people used to C++/Java.

# ? Apr 10, 2009 21:10

POKEMAN SAM: Jul 8, 2004

Janin posted:

It's pretty common in Python code written by people used to C++/Java.

^{[citation needed]}

# ? Apr 10, 2009 21:25

evilneanderthal: Mar 5, 2008; After school we'd all go play in his cave, and every once in a while he would eat one of us. It wasn't until later that I found out that Uncle Caveman was a bear.

Janin posted:

It's pretty common in lovely Python code written by people used to C++/Java.

I hope that provides some clarification.

# ? Apr 11, 2009 02:30

Adbot: ADBOT LOVES YOU

# ? May 13, 2024 23:47

POKEMAN SAM: Jul 8, 2004

evilneanderthal posted:

I hope that provides some clarification.

I've seen my share of lovely Python code and I've never seen that, sorry.

# ? Apr 11, 2009 06:38

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 »