5 ms·

Back in the days at Zynga, there was this ritual that new members of the STG (Shared Tech Group, which developed the game engine stack) had to try to refactor t

Back in the days at Zynga, there was this ritual that new members of the STG (Shared Tech Group, which developed the game engine stack) had to try to refactor the road logic code. Suffice it to say, it's a 28k LOC file that was so bad, it could even hold up in court as evidence that a South American company stole the code of Zynga's -ville games. We could reproduce each and every single bug and its effects 1:1 in their games, with all the crashing scenarios that were easy to reproduce, hard to debug, and almost impossible to fix. Once you dig into the hole of depth sorting and being smart by "just slicing" everything into squared ground tiles on the fly, there's no way out of that spaghetti code anymore. Fun times, was always a joy seeing people give up to a single code file. The first step to enlightenment was always resignation :)

WalterBright 5y ago
It's impossible to refactor spaghetti code without a comprehensive test suite. But you can do it with a test suite - I've done it with large code bases.
- lanstin 5y ago
  You sometimes can. Maybe for any legacy code base someone could, but I have tried and failed on more than one occasion. Some people’s thought process is just perversely different to mine and I keep feeling, oh, this is the layer where that happens, but no every time I have an aha moment I am disillusioned.
  - WalterBright 5y ago
    If you don't have a test suite, you can't know if you're making progress or making things worse. Learned repeatedly from painful experience.
    - manquer 5y ago
      To develop a comprehensive test suite can sometimes be hard, especially for code that deals with say concurrency, multi threaded code , locks , 2d/3d physics , video , analog , hardware related , procedurally generated or ML (meta-language ) and the other ML (machine learning) etc. A lot of edge cases and race conditions would easily slip through, also a different set of edge cases or race conditions you never considered and therefore never tested for in your first version could pop up in your rewrite.
      - WalterBright 5y ago
        Of course. But dealing with that is why we get paid the big bucks. I've dealt with concurrency issues. grep is a handy tool to find related synchronization code, then I try to replace it with an encapsulation. In general, I look for things I can replace with algorithms, and things I can encapsulate. And so on.
      - mbrodersen 5y ago
        It might sometimes be hard but I have never seen a case where it was impossible (25+ years of experience dealing with undocumented legacy code more often than not).
        manquer 5y ago
        If work with not easily deterministic code typically but not always ML models like say speech-to-text or Face-Recognition or classification/ recommendation systems or network performance dependent applications like video conferencing that wouldn't be that feasible . Almost nothing is impossible to test yes, however to know and be able to mock the data for each test case can be extremely hard and at some point not worth the effort to even attempt. Most I have seen these kind of systems doing is statistical testing with reference benchmark/ sample data, and maybe monitor real world feedback either telemetry or user complaints.
        disgruntledphd2 5y ago
        > If work with not easily deterministic code typically but not always ML models like say speech-to-text or Face-Recognition or classification/ recommendation systems or network performance dependent applications like video conferencing that wouldn't be that feasible Nah, most ML systems (actually doing something in the world) are mostly just ordinary code, which can be tested like any other code (as you put it into functions, etc). The models themselves are pretty awkward, but you can normally freeze the model and just use that to ensure that things stay working while you refactor, and then re-run a few (10+) times to check coverage and intervals and stuff. It tends to be more difficult, as many DS/data people are not software engineering focused, but it's not impossible.
- mbrodersen 5y ago
  My experience as well. Also the main recommendation from the most excellent “Working with legacy code” book.
- alkonaut 5y ago
  Which means it's often difficult. The traits "Has a comprehensive test suite" and "is spaghetti code" are often rarely seen together. A poor code base has become poor because it's not refactored and cleaned up all the time - and that's often because their is no tests to help with that. And if there is no test suite, there is often very few ways to add a test suite. Poor code has very few points where you can attach a test. If the code contains file databases or structured input of some sort (a web page) you can add some very high level end to end tests. But not all code has easily verifiable endpoints like that. Perhaps my bad experiences comes from "hard to test" domains (Sound, drawing, ...) code, and not "given this input this is written to the database and this is written to screen".
iaaan 5y ago
To be fair, the first step towards refactoring is understanding the existing code -- ideally, knowing everywhere it is used, all of its behaviors, and importantly, its history, so that you don't break anything, and so that you don't reintroduce bugs that have already been fixed over the years. Or, in lieu of all that, a robust automated test suite. This cannot be done with a file containing 28k lines of code. That is an insurmountable task. They may as well have been asked to start from scratch and build a new engine. I'm curious what the purpose of this ritual was. Was it just hazing, or was the thought that someone might actually be able to accomplish this?
- lupire 5y ago
  A 28Kloc file can be modular.
- hulahoof 5y ago
  I would expect people new to the code base would look at this gigantuan file and ask why it hasn’t been refactored, and this probably was seen as an opportunity to familiarise new hires with the code base as well as get them on the same level as to the effort involved in a refactor. I agree with you, a rewrite is probably how they should have tackled this one.
- z3t4 5y ago
  It is possible to write tests in a single-file program. You could for example have a -test flag that runs all the tests when the program starts. It's never too late to introduce tests - the next bug you fix, start by writing a test that detects the bug, then fix the bug - and confirm that the bug is fixed by running the test. Then you never have to worry about a bug you already fixed will show up again. And the tests will build up to a decent test suit over time. The next step is to also write a new test for each new change or feature. Keep doing this and your program will soon have full test coverage. The trick is to not test small units - instead write the tests as if a user was using the program, so that the actual tests covers a lot of code all over the place, not just the code you added. Mock key presses, and button presses. Then when a user reports a bug, you can write the test so that it repeats the user actions. What makes testing hard is a lot of side effects, like the program writing to databases or calling external API's. Not the LOC of the source file. You might want to mock an automatic call to the fire department for a fire control program, but for API calls and databases, just have the test write to the prod environment, but include rollback/cleanup in the test. That way you don't need a separate testing environment.
deleted 5y ago
[deleted]
gridspy 5y ago
Man, now I want to test myself against your road logic code. Sounds like a worthy challenge. Always tricky though when the hacks have both undefined features AND bugs.
- antifa 5y ago
  This could be like that "endless civ2 game save" where OP thought he was in a permanent stalemate but random internet civ2 veterans found it pretty easy to win.
- tjalfi 5y ago
  If you're looking for obfuscated code to refactor, nroff[0][1] is pretty notorious. [0] http://dtrace.org/blogs/eschrock/2004/07/01/real-life-obfuscated-code/ http://dtrace.org/blogs/eschrock/2004/07/01/real-life-obfusc... [1] https://www.youtube.com/watch?v=l6XQUciI-Sc&t=1h28m35s https://www.youtube.com/watch?v=l6XQUciI-Sc&t=1h28m35s

WalterBright 5y ago

It's impossible to refactor spaghetti code without a comprehensive test suite. But you can do it with a test suite - I've done it with large code bases.

lanstin 5y ago
You sometimes can. Maybe for any legacy code base someone could, but I have tried and failed on more than one occasion. Some people’s thought process is just perversely different to mine and I keep feeling, oh, this is the layer where that happens, but no every time I have an aha moment I am disillusioned.
- WalterBright 5y ago
  If you don't have a test suite, you can't know if you're making progress or making things worse. Learned repeatedly from painful experience.
  - manquer 5y ago
    To develop a comprehensive test suite can sometimes be hard, especially for code that deals with say concurrency, multi threaded code , locks , 2d/3d physics , video , analog , hardware related , procedurally generated or ML (meta-language ) and the other ML (machine learning) etc. A lot of edge cases and race conditions would easily slip through, also a different set of edge cases or race conditions you never considered and therefore never tested for in your first version could pop up in your rewrite.
    - WalterBright 5y ago
      Of course. But dealing with that is why we get paid the big bucks. I've dealt with concurrency issues. grep is a handy tool to find related synchronization code, then I try to replace it with an encapsulation. In general, I look for things I can replace with algorithms, and things I can encapsulate. And so on.
    - mbrodersen 5y ago
      It might sometimes be hard but I have never seen a case where it was impossible (25+ years of experience dealing with undocumented legacy code more often than not).
      - manquer 5y ago
        If work with not easily deterministic code typically but not always ML models like say speech-to-text or Face-Recognition or classification/ recommendation systems or network performance dependent applications like video conferencing that wouldn't be that feasible . Almost nothing is impossible to test yes, however to know and be able to mock the data for each test case can be extremely hard and at some point not worth the effort to even attempt. Most I have seen these kind of systems doing is statistical testing with reference benchmark/ sample data, and maybe monitor real world feedback either telemetry or user complaints.
        disgruntledphd2 5y ago
        > If work with not easily deterministic code typically but not always ML models like say speech-to-text or Face-Recognition or classification/ recommendation systems or network performance dependent applications like video conferencing that wouldn't be that feasible Nah, most ML systems (actually doing something in the world) are mostly just ordinary code, which can be tested like any other code (as you put it into functions, etc). The models themselves are pretty awkward, but you can normally freeze the model and just use that to ensure that things stay working while you refactor, and then re-run a few (10+) times to check coverage and intervals and stuff. It tends to be more difficult, as many DS/data people are not software engineering focused, but it's not impossible.
mbrodersen 5y ago
My experience as well. Also the main recommendation from the most excellent “Working with legacy code” book.
alkonaut 5y ago
Which means it's often difficult. The traits "Has a comprehensive test suite" and "is spaghetti code" are often rarely seen together. A poor code base has become poor because it's not refactored and cleaned up all the time - and that's often because their is no tests to help with that. And if there is no test suite, there is often very few ways to add a test suite. Poor code has very few points where you can attach a test. If the code contains file databases or structured input of some sort (a web page) you can add some very high level end to end tests. But not all code has easily verifiable endpoints like that. Perhaps my bad experiences comes from "hard to test" domains (Sound, drawing, ...) code, and not "given this input this is written to the database and this is written to screen".

iaaan 5y ago

To be fair, the first step towards refactoring is understanding the existing code -- ideally, knowing everywhere it is used, all of its behaviors, and importantly, its history, so that you don't break anything, and so that you don't reintroduce bugs that have already been fixed over the years. Or, in lieu of all that, a robust automated test suite. This cannot be done with a file containing 28k lines of code. That is an insurmountable task. They may as well have been asked to start from scratch and build a new engine. I'm curious what the purpose of this ritual was. Was it just hazing, or was the thought that someone might actually be able to accomplish this?

lupire 5y ago
A 28Kloc file can be modular.
hulahoof 5y ago
I would expect people new to the code base would look at this gigantuan file and ask why it hasn’t been refactored, and this probably was seen as an opportunity to familiarise new hires with the code base as well as get them on the same level as to the effort involved in a refactor. I agree with you, a rewrite is probably how they should have tackled this one.
z3t4 5y ago
It is possible to write tests in a single-file program. You could for example have a -test flag that runs all the tests when the program starts. It's never too late to introduce tests - the next bug you fix, start by writing a test that detects the bug, then fix the bug - and confirm that the bug is fixed by running the test. Then you never have to worry about a bug you already fixed will show up again. And the tests will build up to a decent test suit over time. The next step is to also write a new test for each new change or feature. Keep doing this and your program will soon have full test coverage. The trick is to not test small units - instead write the tests as if a user was using the program, so that the actual tests covers a lot of code all over the place, not just the code you added. Mock key presses, and button presses. Then when a user reports a bug, you can write the test so that it repeats the user actions. What makes testing hard is a lot of side effects, like the program writing to databases or calling external API's. Not the LOC of the source file. You might want to mock an automatic call to the fire department for a fire control program, but for API calls and databases, just have the test write to the prod environment, but include rollback/cleanup in the test. That way you don't need a separate testing environment.

deleted 5y ago

[deleted]

gridspy 5y ago

Man, now I want to test myself against your road logic code. Sounds like a worthy challenge. Always tricky though when the hacks have both undefined features AND bugs.

antifa 5y ago
This could be like that "endless civ2 game save" where OP thought he was in a permanent stalemate but random internet civ2 veterans found it pretty easy to win.
tjalfi 5y ago
If you're looking for obfuscated code to refactor, nroff[0][1] is pretty notorious. [0] http://dtrace.org/blogs/eschrock/2004/07/01/real-life-obfuscated-code/ http://dtrace.org/blogs/eschrock/2004/07/01/real-life-obfusc... [1] https://www.youtube.com/watch?v=l6XQUciI-Sc&t=1h28m35s https://www.youtube.com/watch?v=l6XQUciI-Sc&t=1h28m35s