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 »

ChickenOfTomorrow: Nov 11, 2012; god damn it, you've got to be kind

Are switch statements suddenly considered harmful?

# ? Jan 28, 2013 01:56

Adbot: ADBOT LOVES YOU

# ? May 29, 2024 21:27

zeekner: Jul 14, 2007

Either
-The programmer doesn't know about them.
-They won't work for that situation (incompatible data type, ect)
-Language doesn't support them or has a poor implementation (ie Perl)

That thing is probably a terrifying combination of all three.

# ? Jan 28, 2013 02:10

hobbesmaster: Jan 28, 2008

Salvador Dalvik posted:

Either
-The programmer doesn't know about them.
-They won't work for that situation (incompatible data type, ect)
-Language doesn't support them or has a poor implementation (ie Perl)

That thing is probably a terrifying combination of all three.

I'm hoping its auto generated.

# ? Jan 28, 2013 03:32

cliffy: Apr 12, 2002

Maybe that if else chain was generated somehow. If not I'm sure whoever wrote that immediately threw themselves off a bridge. I know that's what I would've done with myself.

# ? Jan 28, 2013 03:34

kitten smoothie: Dec 29, 2001

tef posted:

http://nsaunders.wordpress.com/2012/10/22/gene-name-errors-and-excel-lessons-not-learned/

Gene names such as MAR1, DEC1, OCT4 and SEPT9 are now reformatted as dates.

Somewhat topical: http://madhadron.com/?p=263

Do we have a bioinformatics thread in these here parts? We should.

# ? Jan 28, 2013 05:24

The Gripper: Sep 14, 2004; i am winner

kitten smoothie posted:

Do we have a bioinformatics thread in these here parts? We should.

We do, it's called the Coding Horrors thread.

# ? Jan 28, 2013 06:24

ultramiraculous: Nov 12, 2003; "No..."; Grimey Drawer

Pardot posted:

look at this code https://github.com/sbrunner/mapserver-tests/blob/master/mapscript/php/layer.c#L269-L326

Wow.

It's technically like a giant unrolled loop...but I feel like that wasn't the intention.

# ? Jan 28, 2013 08:03

Scaevolus: Apr 16, 2007

Pardot posted:

look at this code https://github.com/sbrunner/mapserver-tests/blob/master/mapscript/php/layer.c#L269-L326

C code:

  IF_GET_LONG("numclasses",  php_layer->layer->numclasses)
  else IF_GET_LONG("index", php_layer->layer->index)
    else IF_GET_LONG("status", php_layer->layer->status)
      else IF_GET_LONG("debug",  php_layer->layer->debug)
        else IF_GET_STRING("bandsitem", php_layer->layer->bandsitem)

According to the git history, it got mangled by an automatic code formatter.

It used to look like this:

code:

    IF_SET_LONG("status", php_layer->layer->status, value)
    else IF_SET_LONG("debug",  php_layer->layer->debug, value)
    else IF_SET_STRING("classitem", php_layer->layer->classitem, value)
    else IF_SET_STRING("bandsitem", php_layer->layer->bandsitem, value)           
    else IF_SET_STRING("classgroup", php_layer->layer->classgroup, value)
    else IF_SET_STRING("name", php_layer->layer->name, value)
    else IF_SET_STRING("group", php_layer->layer->group, value)
    else IF_SET_STRING("data", php_layer->layer->data, value)

Which is about as bad as serialization code always looks.

# ? Jan 28, 2013 11:29

Opinion Haver: Apr 9, 2007

Given that it's already macro code, why not turn it into

code:

IF_GET_LONG("numclasses")
else IF_GET_LONG("index")
else ....

since it always uses php_layer->layer?

# ? Jan 28, 2013 11:38

netcat: Apr 29, 2008

C++ code:

static uint16_t ID_NUMBERS[MAX_ID_NUMBERS] = { ID_ERROR, ID_ERROR, ID_ERROR };

Class::Class()
{
  m_all_ID = ID_NUMBERS;
  m_myId = ID_ERROR;

  int i;
  for (i=0; i<MAX_ID_NUMBERS; i++)
  {
    if (m_all_ID [i] == ID_ERROR)
    {
      m_all_ID [i] = START_ID + i;
      m_myId       = START_ID + i;
      break;
    }
  }
}

I'm sure whoever wrote this had a very good reason to not just increment a static variable for the ID, but I just can't see it.

# ? Jan 28, 2013 16:26

Freakus: Oct 21, 2000

netcat posted:

I'm sure whoever wrote this had a very good reason to not just increment a static variable for the ID, but I just can't see it.

Looks like they might be recycling ids.

# ? Jan 28, 2013 20:03

Steve French: Sep 8, 2003

ChickenOfTomorrow posted:

Are switch statements suddenly considered harmful?

That would still be a horror with a switch statement

# ? Jan 28, 2013 20:09

Bunny Cuddlin: Dec 12, 2004

1337JiveTurkey posted:

This thread keeps things anonymous for the most part. I think that makes it significantly different from posting something that identifies the author in a public manner. Even if she does improve her coding style the comments will still be there which is unfair to her if potential employers google her in the future.

One Page Later

Pardot posted:

look at this code https://github.com/sbrunner/mapserver-tests/blob/master/mapscript/php/layer.c#L269-L326

teehee

# ? Jan 28, 2013 20:22

ChickenOfTomorrow: Nov 11, 2012; god damn it, you've got to be kind

Steve French posted:

That would still be a horror with a switch statement

Yes, but it'd have fewer tabs.

# ? Jan 29, 2013 02:55

Scaevolus: Apr 16, 2007

Yes, the macro could be shorter.

You can't switch on a string in C.

# ? Jan 29, 2013 04:50

hobbesmaster: Jan 28, 2008

Scaevolus posted:

Yes, the macro could be shorter.

You can't switch on a string in C.

This is C, the a answer is always more macros.

# ? Jan 29, 2013 04:52

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

Scaevolus posted:

Yes, the macro could be shorter.

You can't switch on a string in C.

Switch on the string hash. :haw:

# ? Jan 29, 2013 05:40

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

Zemyla posted:

Switch on the string hash.

My god, it's beautiful. :patriot:

# ? Jan 29, 2013 14:59

Internet Janitor: May 17, 2008; "That isn't the appropriate trash receptacle."

I linked that string hash example to a friend of mine and the conversation went something like this:

him: "Actually, that's faster than a string comparison."
me: "Does the term false positive mean anything to you?"
him: "So what if it matches on some random characters too? In the real world that doesn't matter- nobody's gonna type garbage in."

:smith:

# ? Jan 29, 2013 15:07

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

Ask him about premature optimization next. Specifically how much processing time is used for command line parsing and also how often it is repeated during the execution of the program :argh:

# ? Jan 29, 2013 15:14

Amarkov: Jun 21, 2010

I don't think it even makes sense to talk about speed or efficiency for operations on five bytes. If you're working in assembly maybe, but Java is almost certainly going to dump that much inefficiency somewhere.

# ? Jan 29, 2013 15:14

Internet Janitor: May 17, 2008; "That isn't the appropriate trash receptacle."

Don't forget that in that example they are hashing an input string, which means they have to walk over each character anyway. It's not even necessarily faster, because in an actual string comparison you can exit as soon as a pair of characters or the size differs.

# ? Jan 29, 2013 15:22

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

I find that pretty much all optimization is premature.

Or maybe "If you think that some operation might be slow, you're optimizing prematurely"

My favorite personal fuckup regarding this was long long time ago I was comparing some things, I want to say in Java. Anyway, the list was something like a million items long and I thought that you know, this might be a slow operation, so I'm going to this thing in batches of 10000 or something and then sleep for a hundred milliseconds or so. You know, to keep the system responsive during the huge load. So the program did its thing and it took maybe half a minute to complete. "Whew, good thing I used that sleep there"

Then I tried it without the sleep, just process the whole list at once. It took maybe 2 seconds.

# ? Jan 29, 2013 15:39

karms: Jan 22, 2006; by Nyc_Tattoo; Yam Slacker

It seems that worrying about legibility before premature optimisation nullifies the latter problem. Plus you can quote the legible version wholesale if you do require murky optimization code.

Internet Janitor posted:

I linked that string hash example to a friend of mine and the conversation went something like this:

him: "Actually, that's faster than a string comparison."
me: "Does the term false positive mean anything to you?"
him: "So what if it matches on some random characters too? In the real world that doesn't matter- nobody's gonna type garbage in."

Aww, that's so cute.

# ? Jan 29, 2013 16:19

1337JiveTurkey: Feb 17, 2005

Internet Janitor posted:

Don't forget that in that example they are hashing an input string, which means they have to walk over each character anyway. It's not even necessarily faster, because in an actual string comparison you can exit as soon as a pair of characters or the size differs.

Mozilla Rhino (and maybe Spidermonkey as well) does an optimized fast path for builtins.

Java code:

    @Override
    protected int findPrototypeId(String s)
    {
        int id;
// #generated# Last update: 2005-09-26 15:47:42 EDT
        L0: { id = 0; String X = null; int c;
            L: switch (s.length()) {
            case 3: c=s.charAt(0);
                if (c=='m') { if (s.charAt(2)=='p' && s.charAt(1)=='a') {id=Id_map; break L0;} }
                else if (c=='p') { if (s.charAt(2)=='p' && s.charAt(1)=='o') {id=Id_pop; break L0;} }
                break L;
            case 4: switch (s.charAt(2)) {
                case 'i': X="join";id=Id_join; break L;
                case 'm': X="some";id=Id_some; break L;
                case 'r': X="sort";id=Id_sort; break L;
                case 's': X="push";id=Id_push; break L;
                } break L;
            case 5: c=s.charAt(1);
                if (c=='h') { X="shift";id=Id_shift; }
                else if (c=='l') { X="slice";id=Id_slice; }
                else if (c=='v') { X="every";id=Id_every; }
                break L;
            case 6: c=s.charAt(0);
                if (c=='c') { X="concat";id=Id_concat; }
                else if (c=='f') { X="filter";id=Id_filter; }
                else if (c=='s') { X="splice";id=Id_splice; }
                else if (c=='r') { X="reduce";id=Id_reduce; }
                break L;
            case 7: switch (s.charAt(0)) {
                case 'f': X="forEach";id=Id_forEach; break L;
                case 'i': X="indexOf";id=Id_indexOf; break L;
                case 'r': X="reverse";id=Id_reverse; break L;
                case 'u': X="unshift";id=Id_unshift; break L;
                } break L;
            case 8: c=s.charAt(3);
                if (c=='o') { X="toSource";id=Id_toSource; }
                else if (c=='t') { X="toString";id=Id_toString; }
                break L;
            case 11: c=s.charAt(0);
                if (c=='c') { X="constructor";id=Id_constructor; }
                else if (c=='l') { X="lastIndexOf";id=Id_lastIndexOf; }
                else if (c=='r') { X="reduceRight";id=Id_reduceRight; }
                break L;
            case 14: X="toLocaleString";id=Id_toLocaleString; break L;
            }
            if (X!=null && X!=s && !X.equals(s)) id = 0;
        }
// #/generated#
        return id;
    }

Honestly I'd just intern the token and do an IdentityHashMap lookup if I'm going to be doing that sort of thing a lot and performance matters.

# ? Jan 29, 2013 16:51

Progressive JPEG: Feb 19, 2003

1337JiveTurkey posted:

Mozilla Rhino (and maybe Spidermonkey as well) does an optimized fast path for builtins.

I'm hoping the 'generated' comments are accurate, in which case that's pretty neato actually.

# ? Jan 29, 2013 17:05

hobbesmaster: Jan 28, 2008

1337JiveTurkey posted:

Mozilla Rhino (and maybe Spidermonkey as well) does an optimized fast path for builtins.

Java code:

    @Override
    protected int findPrototypeId(String s)
    {
        int id;
// #generated# Last update: 2005-09-26 15:47:42 EDT
        L0: { id = 0; String X = null; int c;
            L: switch (s.length()) {
            case 3: c=s.charAt(0);
                if (c=='m') { if (s.charAt(2)=='p' && s.charAt(1)=='a') {id=Id_map; break L0;} }
                else if (c=='p') { if (s.charAt(2)=='p' && s.charAt(1)=='o') {id=Id_pop; break L0;} }
                break L;
            case 4: switch (s.charAt(2)) {
                case 'i': X="join";id=Id_join; break L;
                case 'm': X="some";id=Id_some; break L;
                case 'r': X="sort";id=Id_sort; break L;
                case 's': X="push";id=Id_push; break L;
                } break L;
            case 5: c=s.charAt(1);
                if (c=='h') { X="shift";id=Id_shift; }
                else if (c=='l') { X="slice";id=Id_slice; }
                else if (c=='v') { X="every";id=Id_every; }
                break L;
            case 6: c=s.charAt(0);
                if (c=='c') { X="concat";id=Id_concat; }
                else if (c=='f') { X="filter";id=Id_filter; }
                else if (c=='s') { X="splice";id=Id_splice; }
                else if (c=='r') { X="reduce";id=Id_reduce; }
                break L;
            case 7: switch (s.charAt(0)) {
                case 'f': X="forEach";id=Id_forEach; break L;
                case 'i': X="indexOf";id=Id_indexOf; break L;
                case 'r': X="reverse";id=Id_reverse; break L;
                case 'u': X="unshift";id=Id_unshift; break L;
                } break L;
            case 8: c=s.charAt(3);
                if (c=='o') { X="toSource";id=Id_toSource; }
                else if (c=='t') { X="toString";id=Id_toString; }
                break L;
            case 11: c=s.charAt(0);
                if (c=='c') { X="constructor";id=Id_constructor; }
                else if (c=='l') { X="lastIndexOf";id=Id_lastIndexOf; }
                else if (c=='r') { X="reduceRight";id=Id_reduceRight; }
                break L;
            case 14: X="toLocaleString";id=Id_toLocaleString; break L;
            }
            if (X!=null && X!=s && !X.equals(s)) id = 0;
        }
// #/generated#
        return id;
    }

Honestly I'd just intern the token and do an IdentityHashMap lookup if I'm going to be doing that sort of thing a lot and performance matters.

I would hope that the generated code there was done after a thorough analysis of the problem and reduced execution time the most.

I'm going to assume that I'm wrong, but I can still hope!

# ? Jan 29, 2013 17:05

kitten smoothie: Dec 29, 2001

See also: the HTTP verb parser in nginx.

# ? Jan 29, 2013 18:33

pseudorandom name: May 6, 2007

So how many times has Java changed the String hash function?

# ? Jan 29, 2013 18:37

nielsm: Jun 1, 2009

kitten smoothie posted:

See also: the HTTP verb parser in nginx.

Reading that convinces me that it'd be pretty easy to detect nginx servers just by seeing their reaction to sending the first request line a single byte at a time (Insane TCP overhead go!), and making those requests invalid in some ways.

# ? Jan 29, 2013 19:35

Sedro: Dec 31, 2008

pseudorandom name posted:

So how many times has Java changed the String hash function?

It's specified, so they can't. It's also trivial to generate hash collisions.

# ? Jan 29, 2013 20:03

Malloc Voidstar: May 7, 2007; Fuck the cowboys. Unf. Fuck em hard.

pseudorandom name posted:

So how many times has Java changed the String hash function?

Never have and never will. Java 8 does currently have hash32() on String that returns a 32-bit MurmurHash3 with a seed randomized at VM boot, though.

# ? Jan 29, 2013 21:19

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

it's a shame murmurhash3 has seed/key independent collisions :v:

# ? Jan 29, 2013 21:22

ultramiraculous: Nov 12, 2003; "No..."; Grimey Drawer

kitten smoothie posted:

See also: the HTTP verb parser in nginx.

So what's happening here is it's pre-computing an int/some ints from a list of chars, and then comparing the int to a runtime list of chars, right? So basically you could potentially do a four char comparison in one instruction?

# ? Jan 29, 2013 21:28

Deus Rex: Mar 5, 2005

ultramiraculous posted:

So what's happening here is it's pre-computing an int/some ints from a list of chars, and then comparing the int to a runtime list of chars, right? So basically you could potentially do a four char comparison in one instruction?

isn't this literally what comparing to a hash is?

# ? Jan 29, 2013 21:39

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

Deus Rex posted:

isn't this literally what comparing to a hash is?

No, they don't actually compute a hash. They just pack the bytes into a 32-bit int and directly compare them as such. The actual comparison probably optimizes into one instruction on most hardware.

Edit: or two loads and one compare instead of three or four loads and one compare - something like that. I'm not an assembly master.

# ? Jan 29, 2013 21:54

Carthag Tuek: Oct 15, 2005; Tider skal komme,
tider skal henrulle,
sl�gt skal f�lge sl�gters gang

It checks the length, and then uses macros to check the buffer against n characters by bit-shifting.

# ? Jan 29, 2013 21:54

Zhentar: Sep 28, 2003; Brilliant Master Genius

hobbesmaster posted:

I would hope that the generated code there was done after a thorough analysis of the problem and reduced execution time the most.

I'm going to assume that I'm wrong, but I can still hope!

Any browser code path that is traveled by SunSpider (and I'm guessing that one is hit quite a bit) has been profiled and thoroughly analyzed about 17,000 times.

# ? Jan 29, 2013 21:57

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

tef posted:

it's a shame murmurhash3 has seed/key independent collisions

But since it is randomized between vm restarts, no non-malicious software relies on specific hash values for things, right? So they could just replace it with SipHash?

# ? Jan 29, 2013 22:08

Adbot: ADBOT LOVES YOU

# ? May 29, 2024 21:27

Scaevolus: Apr 16, 2007

Zemyla posted:

Switch on the string hash.

I should have left "barring stupid hash shenanigans" in my post.

Java 7 desugars switch(string) statements into that pattern, but it also adds a check to make sure the two strings match.

# ? Jan 29, 2013 23:16

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 »