51
Root
6y

I wrote a database migration to add a column to a table and populated that column upon record creation.
But the code is so freaking convoluted that it took me four days of clawing my eyes out to manage this.
BUT IT'S FINALLY DONE.
FREAKING YAY.

Why so long, you ask? Just how convoluted could this possibly be? Follow my lead ~

There's an API to create a gift. (Possibly more; I have no bloody clue.)
I needed the mobile dev contractor to tell me which APIs he uses because there are lots of unused ones, and no reasoning to their naming, nor comments telling me what they do.

This API takes the supplied gift params, cherry-picks a few bits of useful data out (by passing both hashes by reference to several methods), replaces a couple of them with lookups / class instances (more pass-by-reference nonsense). After all of this, it logs the resulting (and very different) mess, and happily declares it the original supplied params. Utterly useless for basically everything, and so very wrong.

It then uses this data to call GiftSale#create, which returns an instance of GiftSale (that's actually a Gift; more on that soon).

GiftSale inherits from Gift, and redefines three of its methods.

GiftSale#create performs a lot of validations / data massaging, some by reference, some not. It uses `super` to call Gift#create which actually maps to the constructor Gift#initialize.

Gift#initialize calls Gift#pre_init (passing the data by reference again), which does nothing and returns null. But remember: GiftSale inherits from Gift, meaning GiftSale#pre_init supersedes Gift#pre_init, so that one is called instead. GiftSale#pre_init returns a Stripe charge object upon success, or a Gift (and a log entry containing '500 Internal') upon failure. But this is irrelevant because the return value is never actually used. Pass by reference, remember? I didn't.

We're now back at Gift#initialize, Rails finally creates a Gift object using the args modified [mostly] in-place by all of the above.

Another step back and we're at GiftSale#create again. This method returns either the shiny new Gift object or an error string (???), and the API logic branches on its type. For further confusion: not all of the method's returns are explicit, and those implicit return values are nested three levels deep. (In Ruby, a method will return the last executed line's return value automatically, allowing e.g. `def add(a,b); a+b; end`)

So, to summarize: GiftSale#create jumps back and forth between Gift five times before finally creating a Gift instance, and each jump further modifies the supplied params in-place.

Also. There are no rescue/catch blocks, meaning any issue with any of the above results in a 500. (A real 500, not a fake 500 like last time. A real 500, with tragic consequences.)

If you're having trouble following the above... yep! That's why it took FOUR FREAKING DAYS! I had no tests, no documentation, no already-built way of testing the API, and no idea what data to send it. especially considering it requires data from Stripe. It also requires an active session token + user data, and I likewise had no login API tests, documentation, logging, no idea how to create a user ... fucking hell, it's a mess.)

Also, and quite confusingly:
There's a class for GiftSale, but there's no table for it.
Gift and GiftSale are completely interchangeable except for their #create methods.

So, why does GiftSale exist?
I have no bloody idea.
All it seems to do is make everything far more complicated than it needs to be.

Anyway. My total commit?
Six lines.
IN FOUR FUCKING DAYS!

AHSKJGHALSKHGLKAHDSGJKASGH.

Comments
  • 7
    Every step of the way revealed something else I needed to figure out, do, or build. And many of those did, too.

    Not mentioned: researching the plethora of Redeem/Redemption spaghetti methods, doing database imports, spelunking for unlisted api keys (wtf), Postman auth problems, Postman env setup, buying myself a gift just to test this mess, ...
  • 8
    I’m lost, and I deal with payments and eCommerce 😅

    Oh congrats on 30k 😎
  • 3
    @C0D4 😄😊
  • 3
    @Alice @AlexDeLarge I should probably rant more often, eh?
  • 0
    Time to leak ... huh show the code to a hacker friend to exploit a security flaw ... huh pentest the site.

    Or just advocate for proper coding hygiene and retro-fit it into existing code.
  • 2
    @hugh-mungus in full stack, I get the whole problem 😂
  • 2
    @Fradow This codebase (there are several) has half the line count of war and peace.

    Following it is very much like a non-linear Rube Goldberg machine with multiple inputs and outputs. I don't want to rewrite that. I don't even want to think about it.
Add Comment