
What to Do When Your Colleague Creates Spaghetti Code - douche
https://blog.ndepend.com/colleague-creates-spaghetti-code/
======
CoolGuySteve
The term "spaghetti code" comes from a time when programming languages where
far less structured. Older languages allowed you to jump anywhere in the
program. Modern languages don't let you jump outside of the function scope and
have control constructs like "while".

I'd argue the modern equivalent is the overuse of design pattern abstraction
where every function has one or two lines in it. The symptom is the same:
following the control flow of your program requires a maddening amount of
jumping around in your editor, making it difficult to reason about.

There almost seems to be a cognitive dissonance with some people, where the
terms "GOTOs considered harmful" and "spaghetti code" are thrown around but
they see nothing wrong with hundreds of tiny functions that are only called
from one place. It annoys me that the original point of those terms, that code
is for humans, is lost.

With regards to the article's example, refactoring your code for unit tests is
making life easier for the computer, not humans. And maybe that's why Bill is
annoyed, being human and all.

And don't quote essays you didn't read.

edit: I removed a line but here was the gist of it: Unit tests are great but
many languages don't support them well, especially compiled ones. We need to
strive for a language that let's you isolate a module for testing without the
type system or linker throwing a fit.

~~~
michaelfeathers
The modern equivalent is called 'ravioli code'
[http://c2.com/cgi/wiki?RavioliCode](http://c2.com/cgi/wiki?RavioliCode)

"thousands of little classes everywhere and no one knows how to find the
places where things really happen."

~~~
taneq
I was going to suggest the term "Polenta code".

It's like that, but even smaller pieces. No more than one or two lines of less
than 80 characters each (including indentation).

~~~
NoGravitas
Orzo code? Couscous code?

------
wyldfire
> His code is just… ugly… to the point that you nearly detect a physical odor
> from it.

Unit tests don't magically bring quality code with them. But it does tend to
make it easier to focus on encapsulation.

> "How can we overcome years of inertia, a nasty legacy codebase, ...

Yeah -- this is a real challenge. BTW Bill is not the problem so much as the
codebase. If Bill refuses to consider unit tests, that's a problem that I
think is solveable.

> Simply walking over to Bill ... "You have a tendency to write enormous
> methods...

Nothing would make any designer more defensive than saying "you do [bad
thing]". Bill would probably confess that he, like everyone else, has
contributed a few lines here and there to MegaObject::mega_func(). Your
challenge is that Bill is apparently less motivated than you are to address
the problem. Focus on the code itself.

"Bill, you know mega_func()?"

"OMG yeah that is a nightmare to debug."

"Well, I've been working on unit tests. It took a while but I found a way to
create a unit test to cover MegaObject. Now I can test MegaObject without
deploying to the target first."

"Hmm..."

"Newbie Fred took this test for his first feature, a one-liner fix in
mega_func()."

"Oh yeah?"

"Yeah, and he was able to find and fix issues with his implementation without
going to the lab. And then he refactored mega_func() by adding some
encapsulation to those middle 70 lines. We only need that bit in FROB mode
anyways."

~~~
mi100hael
In my experience, this usually ends with Bill continuing to do what he's
always done and you & Fred trying to continue refactoring and adding tests but
getting frustrated because more untested mega code keeps getting added so your
target keeps expanding. If you're lucky, you'll reach critical mass and be
able to start enforcing coverage or lint checks at the build step (at which
point you can tell Bill "oh yeah you just need to add test coverage and it
will pass" when he comes to you trying to figure out why is build failed), but
more often than not you won't be able to keep up and you'll just get burnt out
& cranky.

------
edw519
_“How can I get Bill to stop writing spaghetti code” is only superficially a
technical problem. In reality, it presents a human problem._

Actually, it's a management problem. Management must establish and enforce
proper process and culture.

Necessary but not sufficient:

    
    
      1. Code standards, determined and agreed upon together
      2. Peer review against those standards
      3. Requirements Definition policies and procedures
      4. Function Specification policies and procedures
      5. Unit Testing policies and procedures
      6. Technical debt policies and procedures (when to grandfather)
    

None of this has to be that complicated. When the rules are understandable,
logical, and everyone agrees to play by them, this takes the monkey off
everyone's back. You don't have to worry about approaching Bill about his bad
code. You have a "business framework" you can rely upon to take care of that
for you. Then no one's the bad guy. This is just good basic I.T. management.

Spaghetti code by others is the norm. I have maintained the shitty code of
thousands of others. If they're gone, I cuss them all day long. If they're
still here, I'm nice. I help management build the processes needed so that I
can stay nice. I do my job. I expect management to do theirs.

~~~
csharpminor
You're absolutely right, but I'll just note that many, many companies do not
have management with the technical acumen to facilitate those decisions. That
kind of leadership is a luxury.

~~~
smonff
Bill is also surely a part of management.

------
overcast
Every time one of these articles surfaces. I feel like everyone on Hacker News
writes immaculate code, in between commenting on new posts.

~~~
DonHopkins
I'm working on Gorgon Medusa [1] code, which used to be quite beautiful, but
now it has snakes for hair and turns you to stone when you look directly at
it, so I have to look at its reflection in my shield while I edit it.

[1]
[http://www.greekmythology.com/Myths/Creatures/Medusa/medusa....](http://www.greekmythology.com/Myths/Creatures/Medusa/medusa.html)

------
kelvin0
There are 2 extremes in the 'code quality' spectrum: A) the programmer who
ships 'stuff that works' but incurs large amounts of technical debt (AKA Bill)
B) the metaprogrammer who will only ship anything 'perfect' and will push back
on any changes he deems to break his crafted code. Of course, there is a
balance to be attained and extreme are rarely the solutions.

~~~
ArkyBeagle
_A_ middle course is to mock things that will support case B). It won't be
perfect, but I think you have to try.

~~~
kelvin0
Not sure I understand your point, care to elaborate?

~~~
ArkyBeagle
I think getting the price down on better solutions takes some measure of
innovation and thinking.

It may be test harnesses. It may be extensive prototyping. It often involves
investing in equipment.

From the coding perspective, it involves having structure be transparent to
function. Everything is explicit; have requirements be represented such that
they're almost derivable from the code base without testing. This doesn't mean
you don't test; it means you do both.

The your concern is the consistency of the written requirements, the code base
and the results of tests.

As an example, I worked on a thing that would cost tens of thousands for one
field test. For about a half or a fourth of the cost of that, I was able to
mock a field test in the engineer's desktop - not in a lab, but right there.
This required a small amount of equipment, about two-four cubic feet worth.
This was based on extensive sets of logged data, translated into models.

Of course it's by definition inadequate; there are lies, damn lies and
simulations. But being able to anneal defects back through the models means
you lose a lot less information.

I eventually put a thing together in a VM that mocked the entire system. I
actually did feature-adds against that, that worked just fine.

I was (briefly) able to show about a factor of ten improvement in defect rate.
But nobody on the team wanted to own this. the implicit message was "we do the
thing; not that thing you just did."

It's now a failing organization, so I'm sure all that got dumpstered. That's
fine; I lit a candle instead of cursing the darkness.

------
Pica_soO
You make interfaces, write everything beyond off as a total failure, and worry
about containment and the impending rewrite of his part later. Its like
managing a burning forest, you cant extinguish it after a certain dimension is
reached, you can only cut the fire from the food and keep it from spreading.

The problem is, when your colleague is supposed to work on the same module and
you are constantly left debugging stupid stuff out of "working code" he wrote.
Management scolds you for being "slow" while the moron gets to play the fast
car.

------
dankohn1
Thoroughly agree that this is a human problem, but I think there are some
amazing automated fixes for it, by incorporating code checkers into the
continuous integration pipeline. Take a look at how the CII best practices
badgeapp [0] uses pronto [1] to run rubocop [2] automatically for free via
CircleCI [3] and surface those results via the Github Status API and email.

The whole point is to avoid having to manually complain in a code review.
Instead, code reviews become a much higher level exercise discussing proper
APIs and code structuring, not sphaghetti issues.

[0] [https://github.com/linuxfoundation/cii-best-practices-
badge/...](https://github.com/linuxfoundation/cii-best-practices-
badge/commit/9dabce047f9a0ab60988aa39ef6524c7e71c7426#commitcomment-18915455)
[1] [https://github.com/mmozuras/pronto](https://github.com/mmozuras/pronto)
[2] [https://github.com/bbatsov/rubocop](https://github.com/bbatsov/rubocop)
[3] [https://circleci.com/](https://circleci.com/)

~~~
ryandrake
But you need to get agreement to implement/work with these automated fixes,
and now you're back to it being a human problem. If you're 3rd engineer from
the left proposing this, your human problem is convincing your team lead or
management to invest in these tools. If you're team lead, your human problem
is convincing the rest of the team the tools' value and getting them to buy-in
to using them.

~~~
dankohn1
Right, but as team lead, there's a huge advantage to implementing the tools
once, and letting individual developers argue with the robot over coding style
(i.e., when they override certain rules), vs. trying to do all code review
manually.

------
triplesec
This is great management, interpersonal, and pragmatic advice for anyone in
many conflict situations, not just in code disagreements. Diplomacy, ego-
conflict reduction, data-orientation and so many more wise human practices are
involved in this.

------
knodi123
> But you’re about to accuse Bill — Bill! — of writing spaghetti code. Bill,
> who started with your company when they had carrier pigeons instead of
> emails, and who singlehandedly built the original version of the software
> with nothing more than COBOL, baling wire and duct tape. Just who do you
> think you are, anyway?

Ugh, I had this problem once. The guy who wrote our company's web store did it
in pure javascript (no libraries) and C (not even c++) on the back end purely
for credit card processing. He didn't believe in googling something that he
could figure out how to do himself, and consequently he never encountered a
single best practice.

In the end, he was so resistant to change that our boss had to fire him. He
went from being the alpha-dog with a decade of seniority who was treated with
utmost respect, to fired, within about two weeks of me volunteering to rewrite
the e-store and the CEO accepting my offer.

On the one hand, I wish I'd known how to handle that more diplomatically. On
the other hand, thank god I don't have to work with such an irritable
primadonna. He was the only employee allowed to work from home, because he was
deathly allergic to computers (no joke), and had a special setup at his house
where the computer was isolated in one room, monitor pressed up against a
window, and only a keyboard and mouse in the room with him.

~~~
Slackwise
> He was the only employee allowed to work from home, because he was deathly
> allergic to computers (no joke), and had a special setup at his house where
> the computer was isolated in one room, monitor pressed up against a window,
> and only a keyboard and mouse in the room with him.

Story aside, I'm intrigued by this. Was he allergic to PCB dust or something?
Or is this some sort of joke or reference wooshing over my head...?

~~~
douche
This sounds like some sort of bizarre psychosomatic thing, like Chuck from
Better Call Saul and his "allergy to electricity"

~~~
knodi123
My take was it was either that, or a bullshit excuse to allow him to work from
home. One the one hand, several other employees assured me his crazy home
setup was legit. On the other hand, he would sometimes have to work a couple
of days in our office during big events, and he seemed fine.

------
michaelfeathers
The thing about unit testing existing code is that it is much easier than most
people think: [https://michaelfeathers.silvrback.com/characterization-
testi...](https://michaelfeathers.silvrback.com/characterization-testing)

~~~
recursive
That doesn't even address the hard part. That link shows how to add tests for
a nice pure function. The reason testing existing code is usually hard is that
it's inserting records in a database or launching nukes or something, and
we're not able to actually do those things in a unit test.

~~~
AnimalMuppet
Of course not, at least not for the launching nukes. Inserting records in a
database? I unit test that all the time. You just need a database on your
development machine, rather than accessing the production database.

~~~
recursive
You need a database, a way to access it, and some kind of guaranteed initial
state, and a way to tell your code to use that stuff instead of the production
database. There's no guarantee that some arbitrary existing code base will
provide a sane way of providing those things.

~~~
AnimalMuppet
An arbitrary existing code base? Perhaps not. The portions I've been working
on for any length of time? They will. They'll have unit tests, too.

Some kind of guaranteed initial state? Yeah, my tests will set that up every
time they run, just like they'll set up everything else they need.

Now, if I'm working on Oracle-specific code, say, and I don't have an Oracle
license for my desktop, I'm unable to do what I have so bravely stated that I
will do. Also, if I'm thrown into a mess with no time to do anything but panic
fixes, I don't have time. But never in my career has that been the case for
long (maybe I'm fortunate...)

------
smonff
Always ask yourself if you don't write _vermicelli_ code before qualifying
Bill of _spaghetti_ coder.

And don't forget that _vermicelli_ should be used in _soup_ , not with
_bolognese_.

Now the question is: will you be able not to write _soup_ code?

------
UK-AL
Make peer code reviews a normal part of the process.

~~~
kelvin0
Great advice, and there are many advantages to doing so. However, my
experience with that tends to demonstrate that programmers will nit pick at
lesser details (function names, coding convention) and not really take the
time to understand what the code on review wants to achieve. Of course, the
latter is time consuming ... any thoughts on this?

~~~
leekh
Having a standards guide or a robo-linters should remove a large chunk of it.
Deferring to a document is much easier then arguing.

~~~
kelvin0
The nit picking I refer to is not about arguing, once a non-standard piece of
code is pointed out, the reviewee usually agrees to correct the issue. I refer
to the fact that the reviewers will look at the leaves in the tree and not
notice the burning inferno and smoke up ahead in the forest...

~~~
MollyR
I found the only thing that helps is having a strong team lead who identifies
this and has the authority to nudge people back on track.

