How to Refactor Spaghetti Code - One Bite at a Time!

Monty Harper - Feb 23 - - Dev Community

This week, I’ve been learning things the hard way!

I’m refactoring my calendar app, partly because I’m applying for jobs now, and I want to present a well-constructed app in my portfolio, but also…

The Problem

More importantly, I’ve hit the “spaghetti point” - that magical moment at which adding, testing, and debugging new features has become difficult because the base code is not so well-organized. And I do have a lot features I want to add or tweak, so I feel it’s worth the effort to do some re-organizing first.

I’ve gotten feedback from a couple of fantastic mentors, one of whom I connected with here on Dev. Daria suggested I should break up my big classes into smaller objects, each of which would have a singular purpose. That’s just good SOLID practice! (I’ll write about SOLID principles another time.)

The Plan

I knew I was stepping into a long dark tunnel by doing things this way, but… I decided to re-write one of my mega-classes FROM SCRATCH!

Luckily, I had already learned one lesson the hard way: think about version control (Git) before launching into a big new refactor. So I…

  1. …committed the project in Git, so I wouldn’t lose any recent changes.
  2. …started a new branch on which to mess things up, leaving a working version of the app as a back-up on my main branch.
  3. …deleted my giant class (called SolarEventManager) and opened a new, blank file.

Now I had a non-working app with a long list of items to replace before it would even build again.

I decided to limit the functionality of my new SolarEventManager to “maintaining an array of solar events.” (Makes sense, right?) I created a separate NetworkManager to actually make the network requests and a ScreenStops struct, which calculates the resulting gradients.

One reason I decided to start from scratch was that I had a lot of code in there I didn’t need at all: a bunch of CoreData stuff (which was a requirement from my Udacity course) when really, UserDefaults would do, plus some code duplicating the function of the system’s location manager. So I needed to replace all that with simpler implementation.

And I also decided to change the way I was making the SolarEventManager available to my views.

So it was a lot.

Stumbling Blocks

I’m not sorry I started from scratch; it was a good way to reinforce everything I’ve learned writing the original code, while solidly (oops, a pun!) comprehending the new structure I was building.

However, it did mean days of typing code with no verification that any of it would actually work!

And please note that I easily could have tackled each of the individual issues listed above one at a time. But no...

Bravely I forged ahead, or… back, I suppose. Finding my way back to ground zero meant hitting three milestones:

  1. Building the app without incurring any errors.
  2. Running the app without crashes.
  3. Getting the app to behave properly.

Because I changed so many things at once, finding and fixing problems at each stage presented a huge challenge. Here are a few of the issues I encountered once I finally got the thing to build again:

  • Crashes due to an @EnvironmentObject not being available in the environment.
  • Custom navigation buttons not working. One of them crashed the app.
  • UserDefault key with a typo, which caused weird behavior and was quite difficult to track down. (I made an enum for those keys and updated the entire app accordingly.)
  • Banner messages not showing up, due to a logic error in some if-else blocks.
  • Crash due to loss of internet.
  • Failure to update when the internet was turned back on.
  • No background view!

That last one stumped me for a while — all my assumptions about where my background went were wrong, wrong, wrong. Turns out, a combination of errors was the culprit:

  • My JSON decoder had the wrong Type,
  • My recursive fetching function wasn’t passing its closure on to the next iteration, and
  • I had a var property defined in the wrong spot so its scope wasn’t what it needed to be.

Those last three bugs were simple oversights. But tracking them down took a WHOLE LOT of detective work, since the error (or errors as it turns out) could have been anywhere in the large swath of code I had changed since the last time things were working correctly.

Lessons Learned

Change one thing at a time. That is all. Change, build, test, fix, commit, change, build, test, fix, commit. Making relatively smaller changes and bringing the app back to working order after each would have helped me isolate problems and fix them more quickly. After all, a brand new problem can only be due to something you just changed, so… yeah, small changes make errors much easier to track down.

The class I deleted was a mess, and I did believe I would end up writing cleaner code by starting from scratch. Maybe that’s true, but also, looking back, to be honest, I think I would have arrived at the same place, and maybe a bit faster. Or maybe a lot faster!

What do you think? Is there ANY advantage to scrapping a whole section of code from a currently working app, and re-building it from the ground up? Or is it always better to change one bit of functionality at a time? Please post your thoughts!

. . . . . . . . . . . . . . . . . . . . . .