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 »

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

FeloniousDrunk posted:

Seems to me all of that above could be written as:
php:
<?
$toret=$userid.'.'.(date('Ymd_His')^'pippopippopippo');
?>
which seems to be making some sort of maybe session key?

Yes and yes. It was changed at some point to the following (which doesn't deserve to be a full function if you ask me)

php:

<?
   $toret = md5($userid . "." . time());
   return $toret;
?>

We have a single point of return policy. It makes for some :downs:

moments in the code. There are also a lot of things done in weird, circuitous ways like using rtrim(ltrim($input)) all over the place instead of trim(). I suspect it's some of the people who have worked on the software were incurious/lazy enough to learn only a small subset of the standard PHP namespace and shoehorn everything into that, but it could also be remnants of the PHP3 where it all began.

# ? Feb 5, 2009 15:43

Adbot: ADBOT LOVES YOU

# ? Apr 26, 2024 06:30

POKEMAN SAM: Jul 8, 2004

Munkeymon posted:

We have a single point of return policy.

I know some people like that, but Ugg I hate it. I'd rather jump the hell out of a triply-nested loop with a Return statement than try to get out some other way. (Though I guess PHP has a break(x) function that breaks you out of multiple loops, doesn't it?)

# ? Feb 5, 2009 20:04

Ciaphas: Nov 20, 2005; > BEWARE, COWARD

Ugg boots posted:

I know some people like that, but Ugg I hate it. I'd rather jump the hell out of a triply-nested loop with a Return statement than try to get out some other way. (Though I guess PHP has a break(x) function that breaks you out of multiple loops, doesn't it?)

I had a coworker once who protested the Single Point of Return policy at my old job by replacing his returns with gotos to the end of the function with the return. He eventually won, on the proviso that he get rid of the gotos :v:

# ? Feb 5, 2009 20:14

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

Ugg boots posted:

I know some people like that, but Ugg I hate it. I'd rather jump the hell out of a triply-nested loop with a Return statement than try to get out some other way. (Though I guess PHP has a break(x) function that breaks you out of multiple loops, doesn't it?)

Yep, but I don't think I've seen it used anywhere I didn't put it in, and for the record, I also think single point of return is stupid. It has ceritanly caused or helped cause some ugly poo poo in our software.

# ? Feb 5, 2009 20:33

Trammel: Dec 31, 2007; .

Just a snippet

code:

     @h=(@h, DocumentTitle($Title));
     ...
     $h[$#h+1]="</tr>\n";
     $h[$#h+1]="</table>\n";
     $h[$#h+1]="</td>\n";
     $h[$#h+1]="</tr>\n";

This combines embedding HTML in the program source, single letter variables with no declared scope, and every line calculates the existing array length, then calculates the last position to add a new element.

# ? Feb 5, 2009 21:09

Zhentar: Sep 28, 2003; Brilliant Master Genius

While investigating an issue today, I noticed a form that should be pure UI takes 4 RPCs to the database server to open. I indulged my curiosity and began investigating.

1. Checks if we can get a lock on a certain record. Does not keep lock. The lock will not be needed at any point in the work flow.

2. Checks if the work flow's context is still valid. It's avoidable, but legitimate.

3. Checks if we are allowed to create new records of a certain type. The work flow does not involve creating any records of any type.

4. Tries to load the name of a record. No record is specified. Doesn't matter because no names are supposed to be displayed anywhere.

#3 seemed especially curious, so I dug deeper. As it turns out, the form is multi-purpose, and there is one situation where new records are created. This situation represents somewhere between 0.01% and 0.001% of the instances where the form is used. The situation is also easy to test for in an if statement, as it is already identified in a boolean variable.

Then I took a look at the server code in the RPC. 7 lines of code, with a logic hole big enough to drive a truck through. It doesn't address the majority of cases at all. Given a certain (unusual) configuration, it will get several cases wrong. Of the few cases it does handle correctly, one was an accident.

With that, I pulled up the documentation for the development. The summary (which is used for release notes) states that these changes were removed.

# ? Feb 5, 2009 21:23

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

Ledneh posted:

I had a coworker once who protested the Single Point of Return policy at my old job by replacing his returns with gotos to the end of the function with the return. He eventually won, on the proviso that he get rid of the gotos

That is loving awesome.

So one of our devs got into a big fight with us (the testing half) about unit testing. He declared that his code was, in fact, unit tested.

This is what he thought constituted unit testing:

code:

            #region VerificationCodePart1
            if (System.Diagnostics.Debugger.IsAttached)
             {

                 if (File.Exists("Log.txt"))
                 {
                     Log = new FileStream("Log.txt", FileMode.Append);
                 }
                 else
                 {
                     Log = new FileStream("Log.txt", FileMode.OpenOrCreate);
                 }

                 Trace.Listeners.Add(new TextWriterTraceListener(Log));
                 Trace.WriteLine("");
                 Trace.WriteLine("********************New Install Selected*******************");
                 Trace.WriteLine("Install Button Clicked");
             }
            #endregion
            buttonCancel.Enabled = false;
            buttonInstall.Enabled = false;
            #region VerificationCodePart2
            if (System.Diagnostics.Debugger.IsAttached)
            {
                if (buttonCancel.Enabled == false)
                    Trace.WriteLine("Cancel button is disabled");
                if (buttonInstall.Enabled == false)
                    Trace.WriteLine("Install button is disabled");
            }
            #endregion

# ? Feb 5, 2009 21:26

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

Multiplication is a slow operation:

code:

#define SOME_CONSTANT 5
int someValue=SOME_CONSTANT << 2;

How to find out a value from an array:

code:

index = 0;
while( ! ( (( current_value - TOLERANCE / 2 ) < valuelist[ index ] ) &&
    ( ( current_value + TOLERANCE / 2 ) > valuelist[ index ] ) ) &&
    ( index < ARRAY_SIZE ) )
    {
        index++;
    }
if( index < ARRAY_SIZE )
{
    /* use value from array */
}

# ? Feb 6, 2009 10:26

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

I was thinking about plunking this in my code:

code:

template < class T > struct NonPtrWrapper
{
	inline T& operator*() { return t; }
	inline const T& operator*() const { return t; }
	inline T* operator->() { return &t; }
	inline const T* operator->() const { return &t; }
	T t;
};

I think it should qualify.

EDIT: also this

code:

struct IndWrapper
{
	inline unsigned int operator[]( unsigned int n ) const { return n; }
};

Painless fucked around with this message at 15:26 on Feb 6, 2009

# ? Feb 6, 2009 15:15

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

Painless posted:

I was thinking about plunking this in my code:

code:

template < class T > struct NonPtrWrapper
{
	inline T& operator*() { return t; }
	inline const T& operator*() const { return t; }
	inline T* operator->() { return &t; }
	inline const T* operator->() const { return &t; }
	T t;
};

I think it should qualify.

EDIT: also this

code:

struct IndWrapper
{
	inline unsigned int operator[]( unsigned int n ) const { return n; }
};

What the hell are you on?

You know you have to include the STL type information into those things, right? Otherwise, it's pointless!

# ? Feb 7, 2009 17:58

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

Zemyla posted:

You know you have to include the STL type information into those things, right? Otherwise, it's pointless!

What are you talking about?

# ? Feb 7, 2009 21:15

POKEMAN SAM: Jul 8, 2004

Avenging Dentist posted:

What are you talking about?

I think it's a joke but idgi

# ? Feb 8, 2009 05:36

ScaryFast: Apr 16, 2003

This isn't a coding horror but it has to do with web hosting and it's horrible.

I just had my first run-in with someone with a simple password. I'm not talking some random unrelated dictionary word, or "God" or "Jesus" but something that seems worse to me.

Customer says all visitors to his site are redirected to some search site. One of our dudes looks around and finds the offending bit of code and says it "looked like an old counter" before removing it. Problem solved.

But the customer replies to the Email to say it's still happening! The code is back.

I emailed him to ask him to call us with his login info on Monday (the next day that I work) so I could fix it, but I also BCC it to the guy in charge of hosting who would probably fix it right away now. I told him in the Email that if this has happened again, someone might have his login info. I mentioned that his password should be complex so that "hackers" can't easily pick the correct one with a dictionary attack. Off goes the Email.

Aforementioned hosting guy messages me on MSN to tell me the password is the company name. ftp.companynamesports.com is the ftp server. companynamesports.com is the username. companyname is the password. Makes me wonder how many people out there have lovely passwords like that. And how many people out there are stealing info from companies because they can log into the FTP server, or have access to a mysql database. I'm just surprised this is my first run-in with something like this. I deal with lots of stupid password issues for Email addresses but never for an account on a web host. Not THIS dumb anyway :P

ScaryFast fucked around with this message at 05:53 on Feb 8, 2009

# ? Feb 8, 2009 05:42

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

ScaryFast posted:

This isn't a coding horror but it has to do with web hosting

awesome, thanks for posting in "Web hosting horrors: post things that aren't coding horrors in any way".

# ? Feb 8, 2009 06:59

Biscuit Hider: Apr 12, 2005; Biscuit Hider

ScaryFast posted:

This isn't a coding horror but it has to do with web hosting and it's horrible.

I just had my first run-in with someone with a simple password. I'm not talking some random unrelated dictionary word, or "God" or "Jesus" but something that seems worse to me.

Customer says all visitors to his site are redirected to some search site. One of our dudes looks around and finds the offending bit of code and says it "looked like an old counter" before removing it. Problem solved.

But the customer replies to the Email to say it's still happening! The code is back.

I emailed him to ask him to call us with his login info on Monday (the next day that I work) so I could fix it, but I also BCC it to the guy in charge of hosting who would probably fix it right away now. I told him in the Email that if this has happened again, someone might have his login info. I mentioned that his password should be complex so that "hackers" can't easily pick the correct one with a dictionary attack. Off goes the Email.

Aforementioned hosting guy messages me on MSN to tell me the password is the company name. ftp.companynamesports.com is the ftp server. companynamesports.com is the username. companyname is the password. Makes me wonder how many people out there have lovely passwords like that. And how many people out there are stealing info from companies because they can log into the FTP server, or have access to a mysql database. I'm just surprised this is my first run-in with something like this. I deal with lots of stupid password issues for Email addresses but never for an account on a web host. Not THIS dumb anyway :P

Congratulations, you deal in customer service.

# ? Feb 8, 2009 17:44

Sergeant Rock: Apr 28, 2002; "... call the expert at kissing and stuff..."

Guys, this isn't strictly a coding horror, but I thought this was the right thread:

How much flour should I use when making dumplings for a goulash? I always have problems with putting too much or too little. TIA.

# ? Feb 9, 2009 10:28

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

Sergeant Rock posted:

Guys, this isn't strictly a coding horror, but I thought this was the right thread:

How much flour should I use when making dumplings for a goulash? I always have problems with putting too much or too little. TIA.

little babby can't follow a recipe

# ? Feb 9, 2009 11:08

zombienietzsche: Dec 9, 2003

Sergeant Rock posted:

Guys, this isn't strictly a coding horror, but I thought this was the right thread:

How much flour should I use when making dumplings for a goulash? I always have problems with putting too much or too little. TIA.

I think your problem is you're cooking in Thickeners on Measures. Real programming languages like cooking++ throw an exception if you try to measure flour by volume.

# ? Feb 9, 2009 12:10

Trammel: Dec 31, 2007; .

Another sample. This was used to determine a new primary key for the Customers table.

code:

sub GetNewCustomerID {
    my $sql = "SELECT CustomerID FROM Customers";
    my $query = Query($sql);

    my(%cust);
    my($highest) = 0;
    while (my $row = $query->fetchrow_hashref) {
        %cust = %$row;
        if ($cust{'CustomerID'} > $highest) { $highest = $cust{'CustomerID'}; }
    }
    return $highest + 1;
}

I've cut the code down to the essentials, but this gets points for:

No usage of the DB's automatically generated primary keys
Selects every row in the customers table and then iterates over them all to find the larges customer id.
Why it copies the contents of each row into another variable? ..
Small chance of a race condition

# ? Feb 11, 2009 03:41

spiritual bypass: Feb 19, 2008; Grimey Drawer

Trammel posted:

Small chance of a race condition

Any time someone who doesn't understand databases gets near one it turns into an incredible horror, that's for sure. It certainly happens for more often than it should. The one thing I don't understand here is where a race condition could occur. How's that work?

# ? Feb 11, 2009 04:49

zombienietzsche: Dec 9, 2003

If a new database row is inserted after the query to determine the max ID has been run, your newly-incremented ID will already exist.

# ? Feb 11, 2009 04:54

Trammel: Dec 31, 2007; .

royallthefourth posted:

Any time someone who doesn't understand databases gets near one it turns into an incredible horror, that's for sure. It certainly happens for more often than it should. The one thing I don't understand here is where a race condition could occur. How's that work?

If you had two applications using this method at same time, there's a chance that they both will select the same highest "free" customer id, before writing a new customer record to the DB.

The longer the delay between selecting the unused customer id and writing a new customer record, the larger the chance that another application come along and use the same customer id, with who knows what results.

# ? Feb 11, 2009 04:55

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

A compelling display of the underlying principle.

# ? Feb 11, 2009 06:09

Aredna: Mar 17, 2007; Nap Ghost

I'm taking a class on Web Development and our book has the following as part of an example on how to use Javascript to test is a string entered by the user is a valid phone number.

code:

function test_phone_number(str)
{
	var ok = str.search(/\(\d{3}\)\d{3} *- *\d{4}/);

	if (ok == 0)
		return true;
	else
		return false;
}

# ? Feb 11, 2009 18:16

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

That's not really that bad. I mean, it's terrible in the "Your code should be figuring out how to format it for me" sense, but the regex itself isn't that bad.

Well, except for the spaces I suppose. What?

# ? Feb 11, 2009 18:35

sonic bed head: Dec 18, 2003; this is naturual, baby!

Ryouga Inverse posted:

That's not really that bad. I mean, it's terrible in the "Your code should be figuring out how to format it for me" sense, but the regex itself isn't that bad.

Well, except for the spaces I suppose. What?

I don't think it's really that good though. First of all, they should be using test instead of search because that returns a boolean. Also, they should use ^ and $ because their regex currently would say true for

code:

(212)123-1234 )(!*@#)(*!@)(*$)(!*@#)(*!

# ? Feb 11, 2009 18:49

Zombywuf: Mar 29, 2008

Ryouga Inverse posted:

That's not really that bad. I mean, it's terrible in the "Your code should be figuring out how to format it for me" sense, but the regex itself isn't that bad.

Well, except for the spaces I suppose. What?

Better hope your web app isn't going to be available in the rest of the world, like if you put it on the internet or anything.

Now just got to hope the book doesn't suggest email regexes like '[a-zA-Z0-9_]+@[a-z]+\.(com|org)'.

Yes I have seen that (or things like it) in production code.

# ? Feb 11, 2009 19:17

Smugdog Millionaire: Sep 14, 2002; 8) Blame Icefrog

I hope it mentions that you can't depend on javascript for validation of input.

# ? Feb 11, 2009 19:51

Erasmus Darwin: Mar 6, 2001

It also fails on "(212) 555-1212". That's pretty screwy considering that it recognizes the possibility of spaces yet fails for the most common place to put a space.

Worst of all is the use of "if (x) { return true; } else { return false; }". What an awful idiom to be teaching to people.

# ? Feb 11, 2009 20:03

Aredna: Mar 17, 2007; Nap Ghost

Free Bees posted:

I hope it mentions that you can't depend on javascript for validation of input.

It doesn't, but our teacher has pointed it out at least 5 times in the last 2 class periods.

Erasmus Darwin posted:

Worst of all is the use of "if (x) { return true; } else { return false; }". What an awful idiom to be teaching to people.

And he threatened to cut off our fingers if he ever saw us do this.

# ? Feb 11, 2009 20:11

geetee: Feb 2, 2004; >;[

Your teacher sounds like a good man.

# ? Feb 11, 2009 23:33

ohgodwhat: Aug 6, 2005

Zombywuf posted:

Now just got to hope the book doesn't suggest email regexes like '[a-zA-Z0-9_]+@[a-z]+\.(com|org)'.

What would be the correct email regex?

# ? Feb 12, 2009 00:05

spiritual bypass: Feb 19, 2008; Grimey Drawer

ryanmfw posted:

What would be the correct email regex?

Something that doesn't restrict you to .com or .org is a good place to start.

# ? Feb 12, 2009 00:08

POKEMAN SAM: Jul 8, 2004

ryanmfw posted:

What would be the correct email regex?

this gets brought up all the time:

(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])

# ? Feb 12, 2009 00:11

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

ryanmfw posted:

What would be the correct email regex?

Its tricker than it seems.
The regex suggested in RFC 2822 (printed below) matches some addresses that will cause most email clients to choke, like name@host.com.fake for example.

(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])

# ? Feb 12, 2009 00:15

ohgodwhat: Aug 6, 2005

I was thinking more along these lines:
http://www.ex-parrot.com/~pdw/Mail-RFC822-Address.html :v:

# ? Feb 12, 2009 00:40

Plorkyeran: Mar 22, 2007; To Escape The Shackles Of The Old Forums, We Must Reject The Tribal Negativity He Endorsed

ryanmfw posted:

What would be the correct email regex?

Sending a verification email to the address they give you.

# ? Feb 12, 2009 00:41

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"

ryanmfw posted:

What would be the correct email regex?

.+@.+

# ? Feb 12, 2009 00:57

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

ryanmfw posted:

I was thinking more along these lines:
http://www.ex-parrot.com/~pdw/Mail-RFC822-Address.html

There is no god

# ? Feb 12, 2009 01:18

Adbot: ADBOT LOVES YOU

# ? Apr 26, 2024 06:30

Mill Town: Apr 17, 2006

tripwire posted:

There is no god

If there was, you could find him with a regular expression of similar length

# ? Feb 12, 2009 01:22

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 »