What is spaghetti code? [closed]

2019-01-25 04:24发布

Can you post a short example of real, overdone spaghetti code, possibly saying what it does? Can you show me a little debugger's nightmare?

I don't mean IOCCC code, that is science fiction. I mean real life examples that happened to you...

Update

The focus has changed from "post some spaghetti code" to "what is exactly spaghetti code?". From a historical perspective, the current choices seem to be:

  • old Fortran code using computed gotos massively
  • old Cobol code using the ALTER statement

13条回答
爱情/是我丢掉的垃圾
2楼-- · 2019-01-25 04:39

This is from a MIDI parser I wrote some time ago. It was a quick and dirty proof of concept, but nevertheless, I shall take the blame for its ugliness: 4 levels of nested conditionals plus the dreaded multiple returns. This code was meant to compare 2 MIDI events in order to sort them by priority when writing to a file. Ugly as it was, it did the job decently, though.

internal class EventContainerComparer : IComparer {

    int IComparer.Compare(object a, object b) {
        MIDIEventContainer evt1 = (MIDIEventContainer) a;
        MIDIEventContainer evt2 = (MIDIEventContainer) b;

        ChannelEvent chanEvt1;
        ChannelEvent chanEvt2;

        if (evt1.AbsoluteTime < evt2.AbsoluteTime) {
            return -1;
        } else if (evt1.AbsoluteTime > evt2.AbsoluteTime) {
            return 1;
        } else {    
            // a iguar valor de AbsoluteTime, los channelEvent tienen prioridad
            if(evt1.MidiEvent is ChannelEvent && evt2.MidiEvent is MetaEvent) {
                return -1;
            } else if(evt1.MidiEvent is MetaEvent && evt2.MidiEvent is ChannelEvent){
                return 1;
            //  si ambos son channelEvent, dar prioridad a NoteOn == 0 sobre NoteOn > 0
            } else if(evt1.MidiEvent is ChannelEvent && evt2.MidiEvent is ChannelEvent) {

                chanEvt1 = (ChannelEvent) evt1.MidiEvent;
                chanEvt2 = (ChannelEvent) evt2.MidiEvent;

                // si ambos son NoteOn
                if( chanEvt1.EventType == ChannelEventType.NoteOn 
                    && chanEvt2.EventType == ChannelEventType.NoteOn){

                    //  chanEvt1 en NoteOn(0) y el 2 es NoteOn(>0)
                    if(chanEvt1.Arg1 == 0 && chanEvt2.Arg1 > 0) {
                        return -1;
                    //  chanEvt1 en NoteOn(0) y el 2 es NoteOn(>0)
                    } else if(chanEvt2.Arg1 == 0 && chanEvt1.Arg1 > 0) {
                        return 1;
                    } else {
                        return 0;
                    }
                // son 2 ChannelEvent, pero no son los 2 NoteOn, el orden es indistinto
                } else {
                    return 0;
                }
            //  son 2 MetaEvent, el orden es indistinto
            } else {
                return 0;
            }
        }
    }
}
查看更多
干净又极端
3楼-- · 2019-01-25 04:41

I'm not pulling this out of my head. This is what I have had to work with, albeit simplified. Let's say that basically you have a program that needs an enum:

enum {
   a, b, c;
} myenum;

But instead what we have is

HashTable t;
t["a"] = 0;
t["b"] = 1;
t["c"] = 2;

But of course, no implementation of a hash table is good enough so there is a local implementation of hash tables, which contains about 10 times more code than an average open source implementation with half the features and double the number of bugs. The HashTable is actually defined virtual, and there's a factory HashTableFactory to create instances of HashTables, but true to the pattern HashTableFactory is also virtual. To prevent an infite cascade of virtual classes there's a function

HashTableFactory *makeHashTableFactor();

Thus everywhere where the code needs myenum's it carries a reference to the instance of a HashTable and HashTableFactory, in case you want to make more HashTable's. But wait, that's not all! This is not how the hash table is initialized, but it's done by writing a code that reads XML:

<enum>
  <item name="a" value="0"/>
  <item name="b" value="1"/>
  <item name="c" value="2"/>
</enum>

and inserts into a hash table. But the code is "optimised" so that it doesn't read an ascii file myenum.xml, but instead there's a compile time script which generates:

const char* myenumXML = [13, 32, 53 ....];

from myenum.xml and the hash table is initialized by a function:

void xmlToHashTable(char *xml, HashTable *h, HashTableFactory *f);

which is called:

HashTableFactory *factory = makeHashTableFactory();
HashTable *t = facotry.make();
xmlToHashTable(myenumXML, t, f);

Ok, so we have a lot of code to get an enum structure. It's basically used in a function:

void printStuff(int c) {
   switch (c) {
   case a: print("a");
   case b: print("b");
   case c: print("c");
   }
}

and this is called in a context where:

void stuff(char* str) {
   int c = charToEnum(str);
   printStuff(c);
}

So what we actually have is instead of

void stuff(char *str) {
   printf(str);
}

we have manged to generate thousands of lines of code (private new, buggy, complex, implementation of hashtables, and xml readers, and writer) in place of the above 3.

查看更多
对你真心纯属浪费
4楼-- · 2019-01-25 04:45

To me, a more modern example of spaghetti code is when you have 20 dlls and every DLL references each other in one way or another. Your dependency graph looks like a huge blob, and your code hops all over the place with no real order. Everything is inter-dependent.

查看更多
Viruses.
5楼-- · 2019-01-25 04:45

Real spaghetti code was done in COBOL and used the ALTER statement.

Here's an example, while listed a "humor", I've seen this kind of thing. Almost got fired once for noting that any program with an Alter statement was obviously in a state of sin. I refused to "maintain" that program, it was quicker to replace it than understand it.

查看更多
霸刀☆藐视天下
6楼-- · 2019-01-25 04:46

Spaghetti code: Originating in the early 60's in Italy as an alternate recipe for certain pasta dishes, spaghetti code was cooked up by one restaurant entrepreneur who attempted to automate the creation of a fool-proof entree. Pressed by the lack of time to complete the design the engineer/chef cut corners which introduced problems in the recipe early on. In a frantic attempt to remedy a good idea gone bad, various spices were quickly added to the concoction as the recipe grew out of control. The result was a stringy, twisty, yet potentially tasty pile of text that would later grow to be a practice cherished by developers world-wide.

查看更多
ら.Afraid
7楼-- · 2019-01-25 04:48

From a Linux SCSI driver (which shall remain nameless to protect the guilty):

wait_nomsg:
        if ((inb(tmport) & 0x04) != 0) {
                goto wait_nomsg;
        }
        outb(1, 0x80);
        udelay(100);
        for (n = 0; n < 0x30000; n++) {
                if ((inb(tmport) & 0x80) != 0) {        /* bsy ? */
                        goto wait_io;
                }
        }
        goto TCM_SYNC;
wait_io:
        for (n = 0; n < 0x30000; n++) {
                if ((inb(tmport) & 0x81) == 0x0081) {
                        goto wait_io1;
                }
        }
        goto TCM_SYNC;
wait_io1:
        inb(0x80);
        val |= 0x8003;          /* io,cd,db7  */
        outw(val, tmport);
        inb(0x80);
        val &= 0x00bf;          /* no sel     */
        outw(val, tmport);
        outb(2, 0x80);
TCM_SYNC:
/* ... */
small_id:
        m = 1;
        m <<= k;
        if ((m & assignid_map) == 0) {
                goto G2Q_QUIN;
        }
        if (k > 0) {
                k--;
                goto small_id;
        }
G2Q5:                   /* srch from max acceptable ID#  */
        k = i;                  /* max acceptable ID#            */
G2Q_LP:
        m = 1;
        m <<= k;
        if ((m & assignid_map) == 0) {
                goto G2Q_QUIN;
        }
        if (k > 0) {
                k--;
                goto G2Q_LP;
        }
G2Q_QUIN:               /* k=binID#,       */

How did I locate this gem?

find /usr/src/linux -type f -name \*.c | 
while read f
do 
    echo -n "$f "
    sed -n 's/^.*goto *\([^;]*\);.*/\1/p' $f | sort -u | wc -l
done | 
sort +1rn |
head

The output is a series of lines listing files ordered by the number of gotos to distinct labels, like the following:

kernel/fork.c 31
fs/namei.c 35
drivers/infiniband/hw/mthca/mthca_main.c 36
fs/cifs/cifssmb.c 45
fs/ntfs/super.c 47
查看更多
登录 后发表回答