51
Root
6y

This codebase reminds me of a large, rotting, barely-alive dromedary. Parts of it function quite well, but large swaths of it are necrotic, foul-smelling, and even rotted away. Were it healthy, it would still exude a terrible stench, and its temperament would easily match: If you managed to get near enough, it would spit and try to bite you.

Swaths of code are commented out -- entire classes simply don't exist anymore, and the ghosts of several-year-old methods still linger. Despite this, large and deprecated (yet uncommented) sections of the application depend on those undefined classes/methods. Navigating the codebase is akin to walking through a minefield: if you reference the wrong method on the wrong object... fatal exception. And being very new to this project, I have no idea what's live and what isn't.

The naming scheme doesn't help, either: it's impossible to know what's still functional without asking because nothing's marked. Instead, I've been working backwards from multiple points to try to find code paths between objects/events. I'm rarely successful.

Not only can I not tell what's live code and what's interactive death, the code itself is messy and awful. Don't get me wrong: it's solid. There's virtually no way to break it. But trying to understand it ... I feel like I'm looking at a huge, sprawling MC Escher landscape through a microscope. (No exaggeration: a magnifying glass would show a larger view that included paradoxes / dubious structures, and these are not readily apparent to me.)

It's also rife with bad practices. Terrible naming choices consisting of arbitrarily-placed acronyms, bad word choices, and simply inconsistent naming (hash vs hsh vs hs vs h). The indentation is a mix of spaces and tabs. There's magic numbers galore, and variable re-use -- not just local scope, but public methods on objects as well. I've also seen countless assignments within conditionals, and these are apparently intentional! The reasoning: to ensure the code only runs with non-falsey values. While that would indeed work, an early return/next is much clearer, and reduces indentation. It's just. reading through this makes me cringe or literally throw my hands up in frustration and exasperation.

Honestly though, I know why the code is so terrible, and I understand:

The architect/sole dev was new to coding -- I have 5-7 times his current experience -- and the project scope expanded significantly and extremely quickly, and also broke all of its foundation rules. Non-developers also dictated architecture, creating further mess. It's the stuff of nightmares. Looking at what he was able to accomplish, though, I'm impressed. Horrified at the details, but impressed with the whole.

This project is the epitome of "I wrote it quickly and just made it work."

Fortunately, he and I both agree that a rewrite is in order. but at 76k lines (without styling or configuration), it's quite the undertaking.

------

Amusing: after running the codebase through `wc`, it apparently sums to half the word count of "War and Peace"

Comments
  • 7
    You will always be Ashkin in my heart
  • 6
    You people are making the transition that much harder.

    Good to be recognised, though 😊
  • 2
    I forgot the obligatory upvote after I commented whoops
  • 3
    Sad about the change, but I have been in this situation before (currently am actually).

    First, eliminate all commented out code. Then start eliminating methods one by one, if the code breaks put it back. Then eliminate all classes whose methods are unused (if there are a bunch of share constants, make a constants class). Then rename all badly named classes, methods, and variables (this part sucks). But then you have a decently clean baseline to rearchitect.
  • 4
    Hey, congrats on the username!
  • 2
    So, I apologize for this pun, but how do I get @Root access?

    I'll see myself out.
  • 2
    @projektaquarius you'd have to be at least as awesome as me.

    @byte-me thanks! 😊
  • 1
    I actually used an assignment in a comparison recently and it actually made the code MORE readable. But that happens once every few years. And I wrote a comment above it. Also, there's this horrible mess that actually works and I'm very proud of f*cking with the compiler this hard: https://codegolf.stackexchange.com/...
  • 1
    Would it be naive to ask if there's any test coverage?
    Once a piece of functionality has tests, it's far easier to start modifying it without worrying what's going to break.
  • 0
    @Fiftyseven There is, but it's been largely ignored lately. In fact, the test suite dies for me due to missing classes.

    @Fabian I concede assignment/permutes within conditionals helps readability in a few instances. `if record.save` and `if (ptr = malloc(data_size))` are two notable, acceptable examples. `if alias = object.data` is not.
  • 0
    @Root pretty sure I am around as awesome as you. I mean did you see my pun?
  • 0
    @projektaquarius pssh. Try harder. 😋
  • 0
    @Root psh. You try harder. 😝
  • 0
    It’s Ashkin!
  • 0
    I am (g)Root!
Add Comment