Have you ever been here? Refactoring old code, seeing new code that partially de-factors it, and taking out my frustrations in code comments.
The old comment for a struct
declaration:
// This structure will be generated // and stay with each connection
The new comment:
// DEPRECATED: This structure has // historically been used as a junkyard // for any data even superficially // connected with "each connection," // resulting in a mass of snot that // has been passed around the whole // entire system, resulting in high // coupling and poorly designed code. // If you need data from this structure, // PASS ONLY THE DATA YOU ACTUALLY NEED, // in a properly designed object or structure. // Please don't share your bodily fluids // with the rest of the system. // // If you see old code that uses data // from this structure, please // refactor it to receive and process // ONLY THE DATA IT ACTUALLY NEEDS. // Please don't share your bodily fluids // with the rest of the system.
A structure with 47 members, including everything from unparsed HTTP to specific parsed command strings for specific commands, that gets passed to every function in the application model and a fair number of functions in the domain model… This is no better than using global data.
We don’t so much care whether or not the data is global. Just because you put your data into a struct and pass it everywhere, that doesn’t improve your design. In fact, some data wants to be global, namely that data that’s actually used by the entire system. Of course, the moment you decide a certain object will be used as-is by the whole system, you find a case where you want two different ones with slightly different behaviors, and then you face a choice: Either factor the single global object into two non-global ones, or give the single global object two different behaviors.
What’s wrong with this second scenario? It’s bad design. Remember the first lesson you learned about design in college? For some of us, it was in high school.
Good Design = High Cohesion + Loose Coupling
In other words, things that work together, keep them close. Things that work separately, keep them apart. This classic rule of good design, first comprehended by the ancients, passed down to us their apprentices, is now fading from the collective understanding.
The bad thing about global data is that it couples code in one part of the system to code in another part. Passing around a massive structure does the same thing. Even passing around a reference to a single variable does the same thing. I once faced a multithreaded system in which a certain object travelled always with a companion, a reference to the critical section that protected it. That system suffered, and still suffers, from race conditions and deadlocks.
Things that work together, keep them close. Things that work separately, keep them apart. Share data at arm’s length and only on a need-to-know basis. And if two pieces of code need to access the same data, put them in the same class, or at least in the same module. And please, please, please, don’t share your bodily fluids with the rest of the system.
-TimK
Yes, I’ve seen this kind of thing. I usually regain my composure and edit the comment before I finally check in the code.
Recently I saw a similar situation where all kinds of unrelated data was put in an object as private with get and set functions for each item. What’s the point of that? Did the programmer think that it was now “object oriented”?
But avoiding things like this actually requires some up front design and planning before diving into coding. And that issue has been a problem for the 30 years I’ve been in this business.
Happy refactoring!
In the comment, you wrote “whole entire system.” That’s redundant, and makes the mass of snot bigger. I suggest “whole system” or “entire system”, but not “whole entire system.”
Harvey, thanks.
Dave, OAOO.
-TimK