Hacker News new | ask | show | jobs
by lugg 4484 days ago
I hope you're not referring to http://pastebin.com/W8B3CGiN

Those 100 line functions wouldn't make it through any half decent code review. Zero modularization, dal is inextricably linked to almost every piece of logic in the thing. Its an object called "Bitcoin" and I have a feeling it is 95% of mt gox' business logic if not all of it, it even writes raw xml to a temp pointer before dumping it into a cache and then making calls to header(). Separation of concerns? What are concerns?

Sure its tabbed neatly, and its got camelCasing, its using some form of orm/query thingy going on in there, but its still one hell of an unmaintainable, untestable mess.

I'd rather a thousand lines of spaghetti than ever having to work on something like that.

3 comments

I once was hired to a gig because of the following words that came out of my face hole: "Some people see a giant hairball of code, and think to themselves 'I don't want to deal with that'. I see the same code and think 'I'm going to get a new boat'."

Bad, evil, horrible code written by misguided children makes the world go around.

Pretty much my job description.
I hope you get paid well.
> Those 100 line functions wouldn't make it through any half decent code review.

True but I have worked on worse. The codebase we inherited from SQL-Ledger is quite a bit worse and was even more poorly commented when we forked (the only comments, aside from two lines describing the module, and copyright notices, were magic comments which if you delete things, stuff breaks).

And some of the modules were 4-5k lines with 1000-line functions. In a codebase of over 100k lines :-P. As we continue to pull out this logic, our codebase increases in functionality and decreases rather significantly in size.

Oh, and it was Perl which relied on sloppy global scoping and so you couldn't test it.

But hey, we are replacing the code at a pretty good clip.

What is really frustrating with the SQL-Ledger codebase (at least as of 2.6 and 2.8, I haven't looked at 3.0) though and is probably the case in the code you are linking to is that the developer had over 10 years of progress in this and still never seemed to manage to improve:

     IS->post_invoice(\%myconfig, \%$form);
Really? That's like pass by reference to dereferenced reference, right?

It's one thing to write crappy software. It's quite another to write it for over a decade and never improve.

I hate to be all, my pile of spaghetti is bigger than your pile of spaghetti, yet I just can't help myself

- 2 decades in development

- just under 3k "scripts", mixture of php html "fusion", php classes, and last decade or so templates have been added

- last wc -l check, was well into 1m lines in house code

- database (we have a few), >300 tables, it used to be around 4 but we culled some tables / systems

This is our oldest system, still in use, still extremely business critical, we're doing a similar thing to you moving things out, cleaning house so to speak. But its going to be ongoing for a long time to come thats for sure.

Some of the people in this thread should get together and start a new dailywtf clone. Pretty sure we could preload a years worth of content with nothing but grep and git-log.

> - just under 3k "scripts", mixture of php html "fusion", php classes, and last decade or so templates have been added

Unfortunately the fact is once you get to a certain point you pretty much have to accept that you are going to have some inconsistency and rewriting is going to happen gradually. With the LedgerSMB codebase, we have a mixture of code before the fork, which is a mixture of Perl spitting out HTML as strings in pieces and Perl assembling db queries in little pieces, so your comment made me somewhat uncomfortable because it reminded me of what I am going through in the rewrite process.

> - last wc -l check, was well into 1m lines in house code

wc -l was in my case closer to 200k, but once you exclude blanks readmes, documentation and comments we added, it got down to about 100-130k. Not really sure.

However, the code was messed up in ways that are only possible by taking powerful features of a language and systematically abusing them. In the db, needless polymorphic associations which served no purpose.

Almost every module had a name of exactly 2 letters, and the scope of some of the modules was just breathtaking. PE.pm for example handled logical groupings of parts for point of sale, customer discount groups, and project accounting.

Now if there were modules that did too much, there were also modules which were divided for no reason at all. You had, for example, ar/ap workflow and user interface split between aa.pl, arap.pl, and arapprn.pl (the latter being printing support functions).

This is code that every time I work with it, I find unpleasant surprises lurking beneath the surface. It is code that when we forked failed basic tests for things like rounding numbers (financial app). We fixed that right away.

If we are lucky we will be done with the rewrite in another five years.

> Some of the people in this thread should get together and start a new dailywtf clone. Pretty sure we could preload a years worth of content with nothing but grep and git-log.

Definitely. I am up for it. Should I start a blog and post the url here? I would be happy to add you.

I personally like the hard coded values and the extremely long line lengths.
That was my first thought too.

That and "this looks kind of structured from the top but the more I look at it, the less structured it becomes."