43
Root
1y

ARGH. I wrote a long rant containing a bunch of gems from the codebase at @work, and lost it.

I'll summarize the few I remember.

First, the cliche:
if (x == true) { return true; } else { return false; };
Seriously written (more than once) by the "legendary" devs themselves.

Then, lots of typos in constants (and methods, and comments, and ...) like:
SMD_AGENT_SHCEDULE_XYZ = '5-year-old-typo'

and gems like:

def hot_garbage
magic = [nil, '']
magic = [0, nil] if something_something
success = other_method_that_returns_nothing(magic)
if success == true
return true # signal success
end
end

^ That one is from our glorious self-proclaimed leader / "engineering director" / the junior dev thundercunt on a power trip. Good stuff.

Next up are a few of my personal favorites:

Report.run_every 4.hours # Every 6 hours
Daemon.run_at_hour 6 # Daily at 8am

LANG_ENGLISH = :en
LANG_SPANISH = :sp # because fuck standards, right?

And for design decisions...

The code was supposed to support multiple currencies, but just disregards them and sets a hardcoded 'usd' instead -- and the system stores that string on literally hundreds of millions of records, often multiple times too (e.g. for payment, display fees, etc). and! AND! IT'S ALWAYS A FUCKING VARCHAR(255)! So a single payment record uses 768 bytes to store 'usd' 'usd' 'usd'

I'd mention the design decisions that led to the 35 second minimum pay API response time (often 55 sec), but i don't remember the details well enough.

Also:
The senior devs can get pretty much anything through code review. So can the dev accountants. and ... well, pretty much everyone else. Seriously, i have absolutely no idea how all of this shit managed to get published.

But speaking of code reviews: Some security holes are allowed through because (and i quote) "they already exist elsewhere in the codebase." You can't make this up.

Oh, and another!
In a feature that merges two user objects and all their data, there's a method to generate a unique ID. It concatenates 12 random numbers (one at a time, ofc) then checks the database to see if that id already exists. It tries this 20 times, and uses the first unique one... or falls through and uses its last attempt. This ofc leads to collisions, and those collisions are messy and require a db rollback to fix. gg. This was written by the "legendary" dev himself, replete with his signature single-letter variable names. I brought it up and he laughed it off, saying the collisions have been rare enough it doesn't really matter so he won't fix it.

Yep, it's garbage all the way down.

Comments
  • 12
    Oh, I remembered some other craziness:

    Overriding column getters on a model -- not pointing to computed columns or anything, but to another object entirely.

    The model (Order) allows you to specify `order.order`, and the column getters pull data from that association instead of the object itself.

    order12 = Order.find(12)

    order12.column_name # reads from order#12

    order12.order = Order.find(26)

    order12.column_name # now reads from order#26 instead!

    Absolutely great for when you need a headache.

    I can come up with absolutely no reason why someone would do this.
  • 4
    Holy crap this is a nightmare!
  • 5
    some of the stuff looks similar to ruby.

    Is it ruby on rails by chance?
    i don't know why, but some of that stuff has this PHP after taste.

    I say that because i have to maintain a rails app too from time to time, and it's glorious sometimes 🤣

    If my boss asks me, why it takes so long, i always say "because the codebase has grown historically", and we both know immediately whats going on 😂
  • 4
    @thebiochemic Yeah, it’s almost entirely Ruby.

    The JS is the worst part of it, by far. Sharing state between partials using global variables, dynamically swapping markup in and out, arbitrary chunks of js rendered to the page in whatever order, etc. Freaking nightmare.

    “Codebase has grown historically.” Love it. Organic code growth, just like a tumor.
  • 2
    @Root yeah that sounds like fun, ugh

    like a tumor describes it quite well haha 😂
  • 2
    I feel bad just reading it. Some of these stuff are trivial. But oh my god approving new code with security issues knowingly just because there is such code already in production 🤦‍♂️
    Creating a field in a mapper object that returns a value from entirety different record of the same type is another 🤦‍♂️
    Are people still trying to generate random strings without using crypto algorithms to generate them? And also why the hell check DB every time if this id exist, just make the column with unique constraint and that’s it.
  • 1
    @abiggersplash I don’t know it, but it does seem kinda fitting.

    @PappyHans A lot of them are honestly pretty minor, but there are just so many it really wears you down.
  • 5
    If people don’t care about following standards and best practices, the you would be fighting the wind mills trying to improve the situation. My previous job was somewhat similar to your situation. They would let the most absurd stuff in production, but will nitpick on variable naming. We we’re quite the opposite of single character vars and functions. An example would be a function name like modifyDateByRemovingSecondsAndConvertToUTCWithNullCheck. God forbid you don’t spell out every line of code in the function name. Needless to say this place sucked the life out of me and I just quit after 6 months. And by the way this is a very well known company with global presence, IPO and all the rest. It is also rated one of the best places to work, but their engineering is 👎
  • 4
    @Root your company work with this metodology, right?

    https://medium.com/@dekaah/...
  • 2
    @zoridan oh my god that’s amazing haha

    They totally do practice eXtreme Go Horse
  • 0
    How much work would it be to rewrite the entire program? How much of code is understandable and well structured, even if only partial?
  • 1
    @max19931 My bet is between 5% and 10% of the code "" ok "".
  • 0
    @zoridan better than 1%, which is what i expected, based on what OP told.
  • 2
    @max19931 The codebase is gigantic.

    Rubocop (a Ruby linter) crashes on several of the models because of their complexity. Several queries needed to be written by hand because the ORM just gives up on the multi-layered polymorphism. Following some code paths take days, without exaggeration.

    There are a few bits of the codebase that are decent and clean and maintainable, but it’s certainly less than 5%, and very likely less than 1%.
  • 0
    @Root OMG THAT UNIQUE ID IS CLASSIC!! These are laughable errors an intern would point out. Hell that “getter” that fetches an entirely different object would be a fireable offense. I would NOT want someone that brain dead on the team...these people better hang onto this job bc they’re not getting hired anywhere even as a junior!! Who the FUCK writes code like this?? This isn’t even bad practice, it’s more like making a game of writing the most complex unnecessary diarrhea that passes bc it somewhat runs. Nobody CREATES a NEW OBJECT for a GETTER!! That’s FRESHMAN COMPSCI!

    The first time I’ve even heard of this crazy ass method is from you, @Root lol. I bet the dev hero is the only one on this planet who wrote a getter like that.
  • 3
    if (!thisObject.isEmpty()) {
    thatObject = thisObject;
    if (thatObject != null) {
    // 8 more levels of nesting
    }
    }
Add Comment