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 »

sarehu: Apr 20, 2007; (call/cc call/cc)

HappyHippo posted:

For 11,000 lines. This is in C#. Ignoring problems with the program's structure, I was able to turn this into about 5 lines of code with reflection.

Then you better start posting your own code too, bub.

# ? Sep 8, 2008 03:15

Adbot: ADBOT LOVES YOU

# ? May 13, 2024 23:02

Victor: Jun 18, 2004

HappyHippo posted:

Hmm

code:

sometype GetInstance(string Reg)
{
      switch (Reg)
      {
            case "first_field":
                  return first_field;
            case "second_field":
                  return second_field;
            case "third_field":
                  return third_field;
            case "fourth_field":
                  return fourth_field;
            case "fifth_field":
                  return fifth_field;
             .
             .
             .
      }
}

For 11,000 lines. This is in C#. Ignoring problems with the program's structure, I was able to turn this into about 5 lines of code with reflection.

Wait, so you have a class with 5,500 fields?

# ? Sep 8, 2008 15:36

ashgromnies: Jun 19, 2004

Just ran across this while adding on some functionality to a project...

code:

my $id        = $req->param($unique_id);

That $id is an identifier given to us by one of our third-party integration partners. We also have different identifiers for users on our side and we map the two to eachother.

So that line grabs the user's third-party ID from the CGI request. Nothing weird, but then 20 lines below that in the same method...

code:

if ($id) {
           my $id   = $user->get_user_id();
           my $user = $self->get_user( $id, 1 );

It reassigns $id to $user->get_user_id() which is OUR identifier for users - and it called it on an instance of $user which it reassigns in the next line with the $id it just retrieved - what the gently caress I'm going to cry.

# ? Sep 8, 2008 18:40

uXs: May 3, 2005; Mark it zero!

Victor posted:

Wait, so you have a class with 5,500 fields?

I'm hoping it's more than one class and he could make into a single method that works for everything.

# ? Sep 8, 2008 18:47

Jethro: Jun 1, 2000; I was raised on the dairy, Bitch!

ashgromnies posted:

Just ran across this while adding on some functionality to a project...
code:
my $id        = $req->param($unique_id);
That $id is an identifier given to us by one of our third-party integration partners. We also have different identifiers for users on our side and we map the two to eachother.

So that line grabs the user's third-party ID from the CGI request. Nothing weird, but then 20 lines below that in the same method...

It reassigns $id to $user->get_user_id() which is OUR identifier for users - and it called it on an instance of $user which it reassigns in the next line with the $id it just retrieved - what the gently caress I'm going to cry.

I don't know much Perl, but doesn't the my in front of $id and $user hide those variables within the scope of the if statement? So if you were commenting this, it might go something like:

code:

if ($id) { #If the 3rd party id is set
           my $id   = $user->get_user_id(); #get our id for the current user, and then hide the 3rd party id so we don't have to worry about it
           my $user = $self->get_user( $id, 1 );#get a copy of the current user

Still disgusting, but perhaps "sensical?"

# ? Sep 8, 2008 20:03

ashgromnies: Jun 19, 2004

Jethro posted:

I don't know much Perl, but doesn't the my in front of $id and $user hide those variables within the scope of the if statement? So if you were commenting this, it might go something like:
code:
if ($id) { #If the 3rd party id is set
           my $id   = $user->get_user_id(); #get our id for the current user,
                                            #and then hide the 3rd party id so we don't have to worry about it
           my $user = $self->get_user( $id, 1 );#get a copy of the current user
Still disgusting, but perhaps "sensical?"

Yeah, it does redefine $id lexically but it's still ridiculous and unmaintainable because at one point $id refers to the third-party ID then in the same method they change it lexically to point to our ID. And they already have an instance of the user - they call get_user_id() on the currently instantiated user, then grab him again.

# ? Sep 8, 2008 20:15

HappyHippo: Nov 19, 2003; Do you have an Air Miles Card?

Victor posted:

Wait, so you have a class with 5,500 fields?

Correct. They have a script to make this file.

For those interested, my solution was something like this

code:

System.Reflection.FieldInfo[] FI = typeof(ThisClass).GetFields();
foreach (System.Reflection.FieldInfo F in FI)
    if (F.Name == reg)
         return F.GetValue(this);

I'm sure there's a better way. It's more of a band-aid, if I could refactor the whole thing there wouldn't need to be any reflection, much less the ~5500 fields.

# ? Sep 8, 2008 21:08

Mustach: Mar 2, 2003; In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.

Use Type.GetField(), don't use Type.GetFields() and loop through (possibly) all 5500 of them.

# ? Sep 8, 2008 21:16

HappyHippo: Nov 19, 2003; Do you have an Air Miles Card?

Mustach posted:

Use Type.GetField(), don't use Type.GetFields() and loop through (possibly) all 5500 of them.

Now that I think about it, that's probably what I did. I was trying to remember without actually having the code to look at.

Edit: VVV Thanks :P

HappyHippo fucked around with this message at 00:48 on Sep 9, 2008

# ? Sep 8, 2008 21:57

POKEMAN SAM: Jul 8, 2004

HappyHippo posted:

Now that I think about it, that's probably what I did. I was trying to remember without actually having the code to look at.

Nice save.

# ? Sep 8, 2008 22:49

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

code:

public ScriptManager ScriptManagerOne
{
    get { return this.ScriptManager1; }
}

Coming across this made me laugh.

# ? Sep 9, 2008 15:29

Zhentar: Sep 28, 2003; Brilliant Master Genius

I've been reluctant to post in here, since most of the things I see would take too much explaining to be funny. But this one takes the cake, and I have to share it:

code:

. s cnt=0
. s cnt=cnt+1,valAry3(cnt)=val3
. s valAry3(0)=cnt
. s valAry4(1)="1"
. s valAry4(0)=1
. k resultvalAry3
. f i=1:1:valAry4(0) q:result1=0  d
. . s result1=0 f j=1:1:valAry3(0) q:result1=1  d
. . . i valAry3(j)=valAry4(i) d
. . . . s result1=1,resultvalAry3(j)=valAry3(j)
. . . . s:pOut(1)="" pOut(1)=pOut(1)_valAry3(j)
. s partResult(2)=partResult(2)!(result1)
. s partResult(1)=partResult(1)!partResult(2)
. s partResult(2)=""
. s partResult("T2")=partResult(1)
. s partResult(1)=partResult(1)&(result2)
. s partResult("T2")=partResult(1)
. s RESULT=RESULT!partResult(1)
. s partResult(1)=""

Note that the leading dots mean this is inside a for loop.

This code is almost equivalent:

code:

. i val3="1" s result1=1 s:pOut(1)="" pOut(1)=val3
s RESULT=RESULT!(result1&result2)

The "almost" is because my version doesn't have a bug in it.

# ? Sep 9, 2008 19:03

zero knowledge: Apr 27, 2008

This is one of my own. It was a file system for a disk simulator written in C for an operating systems class. I was writing a function that would copy disk blocks from cache to a buffer, and it was baffling me because no matter how big the block being copied was, the buffer would only end up with 4 bytes of data in it. I agonized over it for ages before realizing I was allocating the buffer like so:

code:

void *buf = (void *)malloc([b]sizeOf[/b](blockSize))

Where blockSize, of course, is an int that contains the number of bytes in a block. The stupid part is in bold. It took so long to find because I'd read that line and go "right, I'm allocating the size of a block, that makes sense."

# ? Sep 10, 2008 04:08

RegonaldPointdexter: Mar 13, 2006; Hey guys what's going on?

^^^^^
Probably because

code:

malloc(sizeOf(something))

is hardwired in your brain if you're a C coder.

Zhentar posted:

code:

. f i=1:1:valAry4(0) q:result1=0  d
. . s result1=0 f j=1:1:valAry3(0) q:result1=1  d
. . . i valAry3(j)=valAry4(i) d
. . . . s result1=1,resultvalAry3(j)=valAry3(j)
. . . . s:pOut(1)="" pOut(1)=pOut(1)_valAry3(j)

What language is that?

# ? Sep 11, 2008 09:47

narbsy: Jun 2, 2007

RegonaldPointdexter posted:

Probably because
code:
malloc(sizeOf(something))
is hardwired in your brain if you're a C coder.

it's sizeof in C, case sensitive...

# ? Sep 11, 2008 14:19

RegonaldPointdexter: Mar 13, 2006; Hey guys what's going on?

I almost never write C :shobon:

# ? Sep 11, 2008 14:44

Zhentar: Sep 28, 2003; Brilliant Master Genius

RegonaldPointdexter posted:

What language is that?

MUMPS

# ? Sep 11, 2008 15:25

meba rhodium: Aug 25, 2008

Trying to understand even one line of this code base makes me cry. I think I need a shrink.

Hmm, I wonder what's going on here?

code:

ifpf67_ptr = (Ifpf_0067 *) FP_MCOM_CSMSST (p_msdes, 0);

Let's see what FP_MCOM_CSMSST is all about, that should clear things up.

code:

#define FP_MCOM_CSMSST(a,b)  ((void *)    MH_ADDR_ST(FP_MCOM_CSMSHD(a,b)))

Or maybe not. So, what would FP_MCOM_CSMSHD(a,b) be defined as?

code:

#define FP_MCOM_CSMSHD(a,b)  ((Msg_hd *)  &(FP_MCOM_FPMSHD(a,b))[1])

I see. And FP_MCOM_FPMSHD(a,b) then?

code:

#define FP_MCOM_FPMSHD(a,b)  ((FP_msghdr *)(FP_MCOM_BFDA(a,b)))

AAAAAaaaaaaaaa it won't stop! Then what's FP_MCOM_BFDA(a,b)?

code:

#define FP_MCOM_BFDA(a,b)    ((void *)&FP_MCOM_BFHD(a,b)[1])

Jesus loving Christ, how long does this go on?

code:

#define FP_MCOM_BFHD(a,b)    ((Fp_mcom_bfhd *)FP_MCOM_MSBF(a,b))

Nooooooooo...

code:

#define FP_MCOM_MSBF(a,b)    (&((byte *)(a)->heap)[FP_MCOM_MSOFBF(a,b)])

...ooooooo...

code:

#define FP_MCOM_MSOFBF(a,b)  (FP_MCOM_MSSLOT(a,b).ofbf)

...ooooooo...

code:

#define FP_MCOM_MSSLOT(a,b)  ((a)->mstb[(b)])

*drops dead*

I've been trying to wrap my head around this one line of code for hours and I still have no idea. And none of it is commented, in case you feel that ifpf67_ptr, FP_MCOM_CSMSST or FP_MCOM_MSOFBF aren't descriptive enough names for you.

# ? Sep 11, 2008 16:01

deimos: Nov 30, 2006; Forget it man this bat is whack, it's got poobrain!

meba rhodium posted:

I've been trying to wrap my head around this one line of code for hours and I still have no idea. And none of it is commented, in case you feel that ifpf67_ptr, FP_MCOM_CSMSST or FP_MCOM_MSOFBF aren't descriptive enough names for you.

Calling Nebby here to ILLUSTRATE TO YOUR PLEBE BRAIN how SUPERIOR HUNGARIAN NOTATION is.

# ? Sep 11, 2008 16:57

chocojosh: Jun 9, 2007; D00D.

deimos posted:

Calling Nebby here to ILLUSTRATE TO YOUR PLEBE BRAIN how SUPERIOR HUNGARIAN NOTATION is.

Obviously we just don't see the light here. HUNGARIAN WILL SAVE YOU FROM THE DEVIL.

# ? Sep 11, 2008 17:22

Mustach: Mar 2, 2003; In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.

code:

ifpf67_ptr = (Ifpf_0067 *) ((void *) MH_ADDR_ST(
  ((Msg_hd *) &(((FP_msghdr *)(((void *)&((Fp_mcom_bfhd *)
  (&((byte *)(p_msdes)->heap)[(((p_msdes)->mstb[(0)]).ofbf)]))[1]))))[1])));

I need to know what MH_ADDR_ST expands to; I want to see the whole thing, and then see what it simplifies to.

Mustach fucked around with this message at 18:01 on Sep 11, 2008

# ? Sep 11, 2008 17:59

Vanadium: Jan 8, 2005

meba rhodium posted:

I've been trying to wrap my head around this one line of code for hours and I still have no idea.

What does it look like if you just view the preprocessed code?

# ? Sep 11, 2008 18:02

Mustach: Mar 2, 2003; In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.

It looks like what I posted above you. All on one line, of course.

# ? Sep 11, 2008 18:06

meba rhodium: Aug 25, 2008

Mustach posted:

code:
ifpf67_ptr = (Ifpf_0067 *) ((void *) MH_ADDR_ST(
  ((Msg_hd *) &(((FP_msghdr *)(((void *)&((Fp_mcom_bfhd *)
  (&((byte *)(p_msdes)->heap)[(((p_msdes)->mstb[(0)]).ofbf)]))[1]))))[1])));
I need to know what MH_ADDR_ST expands to; I want to see the whole thing, and then see what it simplifies to.

Sure thing, let's see...

code:

#define MH_ADDR_ST(x) (MH_ST_OF(x) ? (char *)(x) + MH_ST_OF(x) : 0)

...

#define MH_ST_OF(x) ((Msg_hd *) (x))->st_of

Vanadium posted:

What does it look like if you just view the preprocessed code?

I still haven't managed to properly run this source file through just the preprocessor without getting an endless amount of include problems. Working on that.

meba rhodium fucked around with this message at 18:15 on Sep 11, 2008

# ? Sep 11, 2008 18:10

RoadCrewWorker: Nov 19, 2007; camels aren't so great

Going bottom up, ive tried to seperate what this does.

php:

<?
//Input: p_msdes
//1. Calulate an index
int index=(p_msdes->mstb[0]).ofbf;
//2. Access p_msdes->heap (a byte array apparently) via that index
A=(&((byte *)p_msdes->heap)[index];
//3. First cast: pointer to Fp_mcom_bfhd
Fp_mcom_bfhd * B=(Fp_mcom_bfhd *)A;
//4. Dereference pointer, add sizeof(Fp_mcom_bfhd) and interpret as pointer to void(?)
void * C=((void *)&B[1]);
//5. Second cast: pointer to FP_msghdr
FP_msghdr * D=(FP_msghdr *)C;
//6. Dereference pointer, add sizeof(FP_msghdr) and interpret as pointer to Msg_hd (?)
Msg_hd * E=((Msg_hd *)&D[1]);
//7. If E->st_of (maybe a pointer?) contains anything but 0, cast E to (char *) (???) and add that,
if(E->st_of != 0)    int F=(char *)E + E->st_of;
//or set 0
else                 int F=0;
//8. Finally, last Cast: pointer to Ifpf_0067. 
Ifpf_0067 * ifpf67_ptr = (Ifpf_0067 *)F;
//If E->st_of is 0 then this is the nullpointer.?>

I hope i didnt reduce/interpret anything wrong, but with code like that i kind of doubt it. Im usually a C# user, so use with caution...

Gotta love inherently obfuscating programming.

# ? Sep 11, 2008 19:01

greased snafu: Jul 17, 2007; I so cool, sandwich and french fries!

From reddit:

quote:

It's just a parser of four levels of message headers, where the start of the payload is either at the end of the header, or at some offset specified by the header.
code:
Fp_mcom_bfhd *pBfhd = (Fp_mcom_bfhd*)((byte *)p_msdes->heap + p_msdes->mstb[0].ofbf);
FP_msghdr *pMsghdr = (FP_msghdr *)(pBfhd + 1);
Msg_hd* hd = (Msg_hd *)(pMsghdr + 1);
char* pCSMSST = hd->st_of ? (char*)hd + hd->st_of : 0;
ifpf67_ptr = (Ifpf_0067 *)pCSMSST;

# ? Sep 11, 2008 20:04

Mustach: Mar 2, 2003; In this long line, there's been some real strange genes. You've got 'em all, with some extras thrown in.

Whoever wrote MH_ADDR_ST() is an rear end in a top hat for writing a macro that re-evaluates its argument three times. "Who needs functions or temporary variables? I know what I'm doing! It's a macro, it'll be efficient no matter what!"

Beaten, but whatever I figured it out without having a clue so I'm posting this diagram:

code:

--------------
|Fp_mcom_bfhd|
--------------
| FP_msghdr  |
--------------
|   Msg_hd   |
--------------
|            |
| /* st_off  |
| bytes */   |
|            |
-------------|
| Ifpf_0067  |
-------------|

# ? Sep 11, 2008 20:21

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

Ugh... why is this page taking forever to load?!?!?

Click here for the full 800x587 image.

oh ..... dammit.

# ? Sep 11, 2008 20:48

trex eaterofcadrs: Jun 17, 2005; My lack of understanding is only exceeded by my lack of concern.

fansipans posted:

Ugh... why is this page taking forever to load?!?!?

Click here for the full 800x587 image.

oh ..... dammit.

That's loving awesome, it's like old COBOL print lines.

I am in awe.

# ? Sep 11, 2008 21:16

meba rhodium: Aug 25, 2008

Awesome, thanks everyone for the help.

One more line down, only a few thousand to go!

# ? Sep 12, 2008 13:40

UberJumper: May 20, 2007; woop

So at my work we use an internal software, called cataloguer. It takes in a single file or folder and a metadata file, that contains a whole bunch of information about the file or folder. Then writes this information to an Oracle server.

Simple right? Well the guy who developed it, left around a year ago. The whole software is only about... 250k lines of C code. The code has 0 comments, and all the variable names have no vowels. And are generally abbreviated, if there is something already with the same abbrevated name. Then he adds a number next to it. So lets say:

buffer becomes! bffr then instead of using lets say a DIFFERENT loving NAME, he uses, bffr001 further away. Now okay, so we just need to make a small change... right? So we shouldnt have to mess with the code.

:suicide:

Theres alot of wonderful hex address attached to variables, and a little bit of ASM code here and there. Why are we doing (void ***)rspu.sttmnt = {0, 0, 0, 0, 0, 1, ........ } What the :suicide:

I dont know what that does. Period. Okay whatever. Huh, looks like thats supposed to be the SQL statement you want to execute, i'm confused. The statement, is an array of, unsigned ints...? :gonk:

Well thats retarded, we should just see how this connects to the SQL server, i guess and write some sort of workaround.

Then we all realized. What SLCT_OFFST = "0x0fb8d2b" does. We realized why we need to install the specific build of the oracle client(7i)...

The system, basically reads the username and password from a plain textfile, starts the oracle client GUI software. Does a shitton, of random memory hacks, and injects the SQL statement we want into the client interface and then it shoves it on the SQL server. I don't know who this man was, but he is a horrible horrible jerk.

:suicide:

We're upgrading to Oracle 11, in two weeks, and the whole work environment rests on this software. This is why the software does extremely werid poo poo, when i try to run it using oracle 11 and just doesn't work. We can't push back the deadline, and i know its our fault. But gently caress.

# ? Sep 15, 2008 22:44

sklnd: Nov 26, 2007; NOT A TRACTOR

I was digging through a 130k line device driver (yeah, wtf) today, and found this gem

code:

PUBLIC S16 voSendMail(icb, pst)

Probably legacy code since this whole thing seems like its several generations of code glommed on one after another, but still funny when that's hanging out in a Linux device driver.

# ? Sep 16, 2008 01:16

and0r: Mar 30, 2005; Cheese it!

Well this is certainly an interesting method of flow control

code:

If someBool = False Then
    If arrID1(iLoopvar) <> lastID1 Then
        GoTo skip
    End If
Else
    If arrID2(iLoopvar) <> lastID2 Then
        GoTo skip
    End If
End If

If 1 = 0 Then
skip:
    [300 lines of code...]
End If

Enterprise Visual Basic 6 :emo:

# ? Oct 2, 2008 06:58

meba rhodium: Aug 25, 2008

This line made me laugh and scratch my head at the same time:

code:

case FMTP_0059:     /* dis-engage cloaking device */

No body, no nothing. Just this line.

# ? Oct 2, 2008 09:44

Cross_: Aug 22, 2008

Now your code is compatible with the Neutral Zone. Congratulations !

Not a code sample, but I just read our company's coding standards:

code:

4) Required: Do not use Hungarian notation for variable names ! 
(..)
8) Optional: For pointer variables prefix them with p, for references prefix them with r.

# ? Oct 2, 2008 20:26

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

That just means don't use full Hungarian with i for integers and sz for null-terminated strings and useless stuff like that, but do note whether something is a pointer or a reference.

# ? Oct 2, 2008 20:43

Cross_: Aug 22, 2008

I understand that, but obviously they realize that descriptive variable names are a good thing, so why stop with pointers/references ? If it's beneficial to know that something's a pointer, then it's just as useful to know that something is an unsigned integer. pSomeText would be okay, but szSomeText wouldn't? Weirdness!

# ? Oct 2, 2008 21:33

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

Cross_ posted:

I understand that, but obviously they realize that descriptive variable names are a good thing, so why stop with pointers/references ? If it's beneficial to know that something's a pointer, then it's just as useful to know that something is an unsigned integer. pSomeText would be okay, but szSomeText wouldn't? Weirdness!

Uh, no. Pointers, references and plain old variables have different semantics and are accessed differently. The difference between an unsigned integer and a signed integer is often accidental - if you just automatically make an int, but you know it'll never get a value less than 0, you might want to change it to an unsigned int later to get more range. Now you either have to rename your iWhatever variable to uiWhatever, or the name no longer matches the type.

# ? Oct 2, 2008 21:49

Vanadium: Jan 8, 2005

So would you prefix a smart pointer object variable with p?

# ? Oct 2, 2008 22:11

Adbot: ADBOT LOVES YOU

# ? May 13, 2024 23:02

Cross_: Aug 22, 2008

JoeNotCharles posted:

Uh, no. Pointers, references and plain old variables have different semantics and are accessed differently.

I would hope that references and plain old variables are accessed the same way- we are talking C++ here after all.

quote:

Now you either have to rename your iWhatever variable to uiWhatever, or the name no longer matches the type.

Ignoring the fact that this betrays bad design- you have to change the type declaration anyway what's the problem with doing search&replace at the same time?

# ? Oct 2, 2008 22:33

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 »