#Refactoring Old Codebase

1 messages · Page 1 of 1 (latest)

toxic haven
#

TL;DR:

When dealing with code that is not yours, what are the most helpful techniques you employ to analyze, maintain and refactor that code, especially if it's in a very poor state when you encounter it?

Curious as to what thoughts people have here in terms of having to do mass-refactorings. I have inherited a pretty rotten codebase. While it worked OK at the time I took it over, it is absolutely rife with coding sins. Some of the stuff you hear people notoriously made fun of online for because it is just so obviously bad. To make things worse, the previous coder is not available (we did not part on amicable terms) and the codebase was horribly "documented". Because its so poorly written, it's often hard to ascertain what something does without spending a lot of time analyzing it, and then weighing out what the unexpected consequences might be of rewriting it.

While it's a great opportunity to get some experience in for refactoring and cleaning code, I'd love to know if anyone here has had to develop any specific techniques for doing so. In preparation I read a bit of "Working Effectively with Legacy Code" by Michael Feathers, "Agile Software Development" by Robert C. Martin, and some online stuff. I also switched from VS Code to Rider which has really helped with giving me a consistent IDE that I can refactor reliably in with its tools. Beyond that, though, it seems the best methodology for refactoring is to just take small, incremental steps toward your goal. Feather's book seems to have some decent techniques but I think they are situational and mostly a set of guidelines to follow that will not fit every problem you have, assuming you have solved some of the more basic issues. Without simply spending time with it and gaining experience slowly, it seems to simply just be a time consuming task that there is no easy solution for.

#

Refactoring Old Codebase

stable vapor
#

At some point refactoring takes longer than rewriting it entirely.

toxic haven
karmic sierra
# toxic haven ## TL;DR: ## When dealing with code that is not yours, what are the most helpfu...

I wonder what kind of code you're even dealing with in the first place. Is this even for unity specifically? Part of me feels like this is for something else.

There isn't much to say about refactoring or cleaning up code here given we can't see what it is or how it's being used. It'd make sense of course to do so in "small, incremental steps toward your goal" but this is how programming is in any case, so this doesn't really help with anything. You write logic, verify it works, then write more logic. If you were starting a new unity project, then yea i'd just consider rewriting it feature by feature as needed.

The ability to rewrite/refactor anything here depends on what you're actually changing. If you're just rewriting a method to not use GameObject.Find 10 times, that's wildly different from changing a core system used in a ton of different places. If you're adding/removing classes or methods, no matter what you do it'll be a pain. You'd need to verify you aren't breaking random logic at every step of the way.
It's worth it to consider if this is just entirely a waste of time. Ultimately, if it works, it works. The last thing you want is to take a month "improving" the code for it to run the exact same way to the end user and not saving you from any actual problem.

toxic haven
# karmic sierra I wonder what kind of code you're even dealing with in the first place. Is this ...

It is Unity. We were working on a game for a few years and the lead programmer we had hired turned out to be writing some really awful stuff. Copy-pasting code all over the place, god-classes galore. I was not experienced enough for a while to call him out on it but by the time I was, we had just finished one of the more difficult features which was communicating with the Steam API for P2P networking (which neither of us had done before) and basically had a vertical slice of every major feature of the game working.

It worked, but it is a house of cards. Every class touches every other class, there is no sense of authority. Booleans are used liberally everywhere. Observer pattern is abused. Information is duplicated everywhere. I'm not sure how to explain it succintly but I can try and find some code snippets to show how bad it is.

#

I guess to take you on a journey, this is a card game. At the beginning of each match with a player, you pick two decks. The first time you boot your game up and get into a match, it works fine. The second time, picking a deck softlocks the game because it was never coded in such a way to deal with the client attempting to run a second game. Certain variables are set, but they are expected to be at the state they would be in if they were completely remade if you launch a new match.

One such example is in our Player object:
public faction? Faction
This gets set and checked throughout various places in our code. This gets set during deck selection, but if it's not null when we enter the deck selection screen, the logic gets confused. I check this in Rider to see where it is being checked:

#

Already a big mess to ascertain why this is needed. I can't simply remove it and expect something not to break, so I opt instead to try and limit the damage by setting it back to null when we exit a game. This avoids having to refactor every instance of it being checked, but it's only a guess that setting it to null won't break something else. I proceed.

The deck selection screen now proceeds further, but now when I select a deck, I get a new error: my Faction variable was accessed while null. This doesn't happen the first time during a playtest, only the second time. I dig into the issue further and find out where it's being set, and I find this:

idle pewter
#

You‘re only wasting time if you try to repair a system you think is badly constructed

#

Doesn’t even matter if that’s true or not. If you disagree with it you can’t maintain it productively. All the best practices boil down to: wrap it, slice it, replace slices incrementally. If that’s not possible or if noting yet depends on it (clients/customers): rewrite

#

There is nothing worse than maintaining a pile of garbage that you had the opportunity to replace but didn’t. (Team morale etc.)

#

Small caveat: that code you shared looks typical for a relatively naive but dedicated effort. I’ve seen successful games that have code like this. So it can work, but only if the author(s) stick(s) around. It’s usually not possible to pass it on to a new person/team efficiently.

toxic haven
# idle pewter Doesn’t even matter if that’s true or not. If you disagree with it you can’t mai...

It's definitely not impossible to refactor it, but it is very time consuming for me at my skill level, and I'm wondering if it's just a lack of skill. I'll admit, replacing a lot of this stuff wholesale is an intimidating prospect, especially considering we have customers waiting to have a full version of this game some day.

I've reasoned that, I could just ship of theseus this thing until it's manageable, piece by piece. It may be all that I'm capable of at the moment. A rewrite could be even more time consuming and more problematic, and it seems like either way I slice it, I'm in for a lot of work to get the code in a state where the things I need to change are changeable, and I just need to get better at coding either way. At least this way, I have a model to work off of that can be refactored as opposed to building anew. But it certainly feels like it's a long and slow process, and people seem to feel like starting anew is a scary, but ultimately more efficient time investment.

toxic haven
idle pewter
karmic sierra
#

Yea this doesnt really sound like something to refactor based off the sole statement you said of it not being able to run a 2nd time. I think no matter what you call it, refactoring or rewriting, at the end of the day a lot of the code would have to change.

toxic haven
#

You can peruse it there if you are so inclined. A lot of code is written like this.

Ultimately the problem is that it's not just one variable I need to worry about, it's another variable inside the Player class named deck1Picked. Both get set during this process, and if deck1Picked is set, it skips a bunch of logic and breaks the sequence of events.

toxic haven
# idle pewter Is it code that if you change one structural thing that change cascades into man...

I wouldn't say it's weeks of refactoring for any individual thing, but to, for example, remove all of these dumb switch statements that have divergent logic in just about every method of this class, I'd probably have to construct some big interface that includes these methods and then simply create an AI and a PVP version of each with their own logic in each subtype. That would just make the code cleaner to read, but it wouldn't remove the duplicated code, and it definitely could go very wrong.

My feeling on it though is that it's really just a skill issue. If I were a good enough coder to start anew, then rewriting this existing code would probably not be a big deal anyways, but I guess that's what I am here to find out.

#

I apologize if this is a difficult question to answer without a lot of specifics and a lot of reading into the problem. I don't get to interact with coders much and I'm almost entirely self-taught, which is why I sometimes come to places like this to try and get a feel for the landscape and sanity-check myself.

karmic sierra
# toxic haven I apologize if this is a difficult question to answer without a lot of specifics...

there isnt much to say about how to refactor/rewrite when talking in theory. Its just experience needed to take a bad piece of code and write it in a better way. Any advice would really come down to basic programming skills like the first thing you mentioned "small incremental changes". The most i can say is look into SOLID if you want to know more about design. Though no amount of reading is going to rewrite those switch statements for you.
People can also suggest patterns or such to help based on specific code you show.

toxic haven
# karmic sierra there isnt much to say about how to refactor/rewrite when talking in theory. Its...

This is kind of in line with what I've thought in the first place. I feel that if refactoring this code is such an issue, it's probably due to a lack of experience in knowing what a better model would look like, and in many cases in life, there's not much of a magic methodology to use. I will say Working Effectively with Legacy Code did help with some techniques in showing how small changes can be made safely but beyond that, it seems down to your own experience on whether or not you can get the job done, and it requires a strong body of knowledge on the subject to do well.

#

I will look into SOLID like you mentioned.

idle pewter
# toxic haven This is kind of in line with what I've thought in the first place. I feel that i...

I would say your mindset is certainly an indicator that you can actually improve the codebase (given time), but its still also important to recognize when the actual business expectations are not compatible with your actual skill/experience level. But you have to gauge that for yourself. No matter how much of a safety margin your add to your estimates, you will still be off by a factor of two to six.

#

can you give a 100 word summary what the game's scope is from which one could estimate the development effort? how much time and resources do you actually have?

toxic haven
idle pewter
toxic haven
# idle pewter can you give a 100 word summary what the game's scope is from which one could es...

The game is a deck-based card game. decks are pre-determined, 10 in total by 1.0. Each deck is 40 cards, with perhaps 14 unique varieties. At the start of a match, each player picks 2 out of the ten decks. The game plays probably more similar to Magic the Gathering or Yu Gi Oh than something like Hearthstone. The game is to have PVP (and it does currently) over Steam's networking. It has a lobby list to navigate to find players. Each deck operates under a similar set of rules but most have some specific logic that doesn't apply to the others. Eventually the game will have matchmaking, but we're not sure exactly how that will get implemented.

toxic haven
# idle pewter so 1-4 years?

We're closing in on year 3. When we started I barely had enough experience to complete Unity's training course on scripting.

idle pewter
#

i would say your way in over your head.

#

by a mile

toxic haven
#

Certainly feels that way, lol.

#

Well, I don't know, I've been at this alone for months now and it doesn't feel quite that bad.

#

I wouldn't say it's abysmal, it just sucks, and everything takes longer than I'd like.

idle pewter
#

really anyone would be in over their head to make a competitive deckbuilder without having at least a couple of released 'regular' games.

toxic haven
#

I will say if we knew how difficult this would be, we never would have started with a card game.

idle pewter
#

you can make one, and release it, but i take it you actually want to make a good one?

toxic haven
#

We would have stuck with something basic, like a little small-scope platformer or something that we could get quick feedback on but there were reasons we decided to do this that we thought might outweigh the difficulty.

idle pewter
#

deckbuilders are one of the most difficult programming problems in games 😉

toxic haven
#

I think in hindsight we all realize that it was not worth it, but we've kind of gone too far to stop now.

idle pewter
#

and you added online multiplayer 😉

toxic haven
#

Yeah.

idle pewter
#

which is the other difficult programming problem

toxic haven
#

I've definitely learned a lot about both throughout this process.

idle pewter
#

now you just need to add physics 🙂

toxic haven
#

lol no thanks

#

Either way, giving up isn't really an option. It's not like we need to release by the end of the year or anything. Time is flexible. This is a venture we knew might not go how we expected but no one gets anywhere by not trying to do anything interesting or risky.

idle pewter
#

its never too late to stop

toxic haven
#

Well, money has been spent. Something needs to get delivered, even if it takes a long time to do it.

idle pewter
#

or rather, today is the best day to stop doing the wrong thing.

idle pewter
toxic haven
#

That's not really an issue atm. I'm not going to starve or anything.

idle pewter
#

well, if you have infinite time, you can definitely push through

toxic haven
#

"Infinite", yeah. But you know, obviously, the sooner the better.

idle pewter
#

for that reason, you should do the right thing, cause nothing keeps you from doing it

grand tide
#

My advice would be to get what you have to a functioning state and release. Then get started on the sequel

toxic haven
#

I definitely don't want to be attached to this for longer than needed.

idle pewter
#

if you have no reputation to loose, you can just release whatever you have and call it done.

grand tide
#

No one is going to remember your first crappy game

toxic haven
#

It might not be an amazing game. We're not really concerned about it being the best thing ever, but there are some people who want it, and we want to at least be proud of what we've done, however quaint it may be.

idle pewter
grand tide
#

Everyone's first game(s) are crappy

idle pewter
toxic haven
# idle pewter well, just know that this is a dangerous attitude that gets you exploited.

I get what you are saying. There's no exploitation going on here. I didn't just sign up with a random group and then decide I'd go with them to the bitter end. Our expectations were manageable from the beginning but we're all resolute on getting this game made in some form, and we think the end result will be fun enough to warrant the effort. And it'll be a great stepping stone onto something better.

#

It just sucks that I have to wade through this shitty code to do it lol

idle pewter
#

i mean there are investors who prey on this kind of attitude 😉

toxic haven
#

I think at some point in my career I will need to wade through code I don't like, but I was hoping people might have some specific advice to give on how to do it that I might be missing, but I think my suspicion is that there is no one great way that beats just knowing how to code well in the first place.

#

I suspect I will never have to wade through code this bad ever again. I could be wrong though!

idle pewter
#

nobody really cares how crappy the code in your previous project was and how much you have suffered.

#

personally i'd rather work with someone who has seen what the good stuff (successful/maintainable) looks like

toxic haven
#

Well, maybe when I move onto the next thing we'll try and set that up. Properly this time. I could learn a lot from someone who knows what they're doing, but I'm going finish the game I've got for now, and I'm probably going to be doing it alone.

#

I appreciate the advice. It's good to gauge things from other people sometimes.

idle pewter
#

when you finish this slog, you'll be the expert 🙂

toxic haven
#

That's the goal.