Software development is a brilliantly diverse field. With countless voices and perspectives, multitudes of open source projects have thrived despite existing in complex and rapidly changing ecosystems. I’d argue the general reason for this is two-fold:
- The majority of the open source devs do it for the love of the game.
- Higher diversity of experience in open source.
Now of course, not all open source is perfect. I’ve seen plenty of projects lacking maintainers, hosting horribly written code, disorganized to all shit, you get the picture. The benefit of open source, though, is you can freely choose what you work on, and can navigate through whatever sludge there may be to find a project you are truly passionate about — where your voice matters.
This doesn’t extend to businesses. In fact, it tends to be the opposite experience, at least for me. It’s real unfortunate that somehow, in my many internships and jobs, I cannot for the life of me work somewhere that doesn’t have the worst damn code imaginable. So, forgive me for absolutely despising enterprise software, because it appears to be constantly infested with LEGACY CODE.
I feel like there’s some general dos and don’ts that people sometimes miss or forget about when working on software. Sometimes they’re little things that don’t really impact the code, but can affect readability. Other things are questionable design choices. Sometimes they’re even design choices made by the majority of software companies in the world (i.e., the worst fucking ideas imaginable). And you’d think we’d learn and improve thanks to the mistakes of our predecessors right?
Haha.
The little things
Let’s start with some minor things that really don’t matter but still piss me off. Let’s take a peek at the innocent little function below that checks if a feature flag is enabled.
bool IsFeatureEnabled(Context ctx, string key)
{
if (_security.HasPermissions(key, ctx)) // Returns a bool.
{
return true;
}
return false;
}Every line of code counts. Let’s dissect this a little because apparently we have to do that to write something reasonable. This function is essentially a wrapper around a more complex function. The existence of this alone isn’t entirely unreasonable. Although the HasPermissions function on the _security object is pretty self-explanatory, think of a case when an API has a much more complex public interface. This function isn’t exactly where my problem lies.
You see I have the fundamental belief that code should describe, in its own form, what it’s doing. This function is a wrapper around the public interface of the _security object. That is the sole purpose of this function. The obvious way to write a very simple wrapper is to just return up the results of the wrapped call.
bool IsFeatureEnabled(Context ctx, string key)
{
return _security.HasPermissions(key, ctx));
}The key benefit to writing it this way is that simplifying this function makes it easier to argue the need of the function. If we have a wrapper around one line of code, do we even need this function? What benefits does this wrapper provide?
Now I’ve seen a counter to this with the typical “what if we want to put a breakpoint?” or “what if we want to add logging or some other logic here?” to which I reply:
- Breakpoint argument is irrelevant. You can place breakpoints on a return line.
- If you need logging then you can change the function to the old syntax and add logging then. In same cases however you wouldn’t even need to branch, you could just log the result and then return.
But I can hear some already groaning and rolling their eyes. “Who cares? It’s such a minor issue.” You’re absolutely correct my dear fellow! It only saved us four lines, this is nonsensical! Until, of course, this habit is repeated for an entire file tens of tens of times, which quite literally can add up to hundreds of lines of purposeless code.
This is the kind of shit I’m talking about. I feel like people don’t take these minor issues seriously because they are complacent with working on horrendously designed codebases that have files thousands of lines long. Every job I’ve worked at there are those files, sometimes in the numbers of hundreds or more. Thousands of lines, thousands of useless comments, thousands of unoptimized, low effort code written that just makes my job a complete pain in the ass rather than the joy I know programming can be.
But I hear ya. My suggestion above must certainly be overcomplicated. Let’s try applying this style to similar functions like property getters to see if we can improve the old code.
Before:
class Foo
{
private bool _bar;
public bool Bar { get; };
}After:
class Foo
{
private bool _bar;
public bool Bar
{
get
{
if (_bar)
{
return true;
}
return false;
}
}
}Ah. Perfection. Certainly nothing wrong with that!
Maybe we should all do this. Would certainly make us harder to lay off. No one will know what the damn code does except the original writers!
While on the topic of useless lines of code, here’s another thing that pisses me off that I think most developers can relate to.
public int calculate(int x, int y, int z)
{
// Calculate (x + y) / z. Note z must not be 0! Returns a -1 if result is greater than 100.
// var result = x + y + y;
// return x + y + z;
#region Calculate sum
var res1 = (x + y);
var res2 = z;
// calculate
var result = res1 / res2;
#endregion
#region Verify result is within parameters.
if (Math.Abs(result) > 100) // get absolute value of number
{
result = -1;
}
#endregion
//return the result
return result;
}Woah! That is… shit! This heap of junk is a complete fustercluck and unfortunately is the average experience when navigating a majority of legacy systems, especially in enterprise code. Where should I start? How pointless the regions are? How unnecessary the comments are? How complicated the function is?
I’m personally a fan of ternary operators but I can see the argument for avoiding them. Either way, there are certainly better ways to tackle this:
/// <summary>
/// Calculate (x + y) / z, bounded by 100.
/// </summary>
/// <returns>
/// (x + y) / z, or -1 if the absolute value of the
/// calculation yields a value greater than 100.
/// </returns>
public int calculateIfElse(int x, int y, int z)
{
var res = (x + y) / z;
if (Math.Abs(res) > 100)
{
return -1;
}
return res;
}
/// ... same as above
public int calculateTernary(int x, int y, int z)
{
var res = (x + y) / z;
return Math.Abs(res) > 100 : -1 ? res;
}There are stylistic liberties you can take here. There’s other ways to write this. And for the love of all things, yes, I understand that what makes code “clean” is subjective and not suitable for all people. That being said, I feel like you’d be lying to me if you said the code from above is better than this. If your argument is that this feels like a strawman, then all I have to say is you have not worked on enough legacy/proprietary/enterprise systems.
Bigger Problems
We’ve gotten away from some of the minor nits and moved onto actual design issues that can affect the development of the application long-term. You’ve definitely seen these complaints before: it’s Agile/OOP overload.
Despite what you may think given my strong stances and tone, I actually like things like Agile and OOP for certain use cases. Like programming languages, these things are tools and serve a purpose to us: getting a product/library to some end user. There are times when these are very useful tools, and other times (every single enterprise app ever) where it becomes a complete nightmare to manage.
“Sarcasm is the lowest form of wit, but the highest form of intelligence.”
Oscar Wilde
Let’s embed ourselves in the aura of Uncle Bob. Let’s indulge in the essence of true Agile/OOP. Not any of that bullshit that we see researchers and engineers using to research message passing via encapsulated objects in strict hierarchies. No, I mean the real OOP, the enterprise OOP.
Forgive my general lack of knowledge regarding C# console apps. Let’s switch to C++. Let’s assume that our customer wants a basic C++ app that reads in user input and outputs it back to them. We can start with our basic scaffolding that will be the foundation of our project.
#include <iostream>
#include <string>
auto main() -> int {
auto input = std::string();
std::cin >> input;
std::cout << input;
return 0;
}… huh. Well that seemed easy enough! Ah, but we can’t merge this code in yet because we need to have a demo with our product manager (PM). Fair enough. In the meantime, we can apply some changes that our principle engineer has suggested. Let’s modular-ize this to support additional functionality in the future.
#include <iostream>
#include <string>
#include "company/lib.h"
class StreamReader {
protected:
auto read_data_input_from_stdin_nodelim_via_std_cin() -> std::string {
auto s = std::string();
lib::read(s); // internally calls std::cin
return s;
}
};
class StreamWriter {
protected:
auto write_data_to_output_buffer_via_std_cout(std::string s) -> void {
lib::write(s); // internally calls std::cout
}
};
struct UserContext { /* ... */ };
class IApplicationRunContext {
public:
virtual ~IApplicationRunContext() = default;
virtual auto get_user_ctx() -> UserContext = 0;
};
class ApplicationRunContext : public IApplicationRunContext {
public:
ApplicationRunContext() : _ctx({}) {}
auto get_user_ctx() -> UserContext override {
return _ctx;
}
private:
UserContext _ctx;
};
// Application facdae.
class IApplicationStreamProcessorFacade {
public:
// Function is virtual for polymorphism. See link below
// reference: h
virtual ~IApplicationStreamProcessorFacade() = default;
// Run the application.
virtual auto run(IApplicationRunContext ctx) -> void = 0;
};
class ApplicationStreamProcessor : public IApplicationStreamProcessFacade {
public:
// Run the application
auto run(IApplicationRunContext ctx) -> void {
auto stream_reader = ApplicationStreamReader();
auto stream_writer = ApplicationStreamWriter();
// read & write
stream_reader.read();
stream_writer.write();
}
protected:
class ApplicationStreamReader : public StreamReader {
public:
auto read() -> std::string {
auto result = this->read_data_input_from_stdin_nodelim_via_std_cin();
return result;
}
};
class ApplicationStreamWriter : public StreamWriter {
public:
auto write(std::string s) -> void {
this->write_data_to_output_buffer_via_std_cout(s);
return;
}
};
};
auto main() -> int {
auto app = ApplicationStreamProcessor(UserContext { /* ... */ });
app.run();
return 0;
}Now clearly this code here could be improved. We don’t have a factory for the ApplicationStreamProcessor or a singleton to represent the global instance of the stdio handler. Is that even a real thing? Doesn’t matter, the PM has asked for it so that’s what we’ll work on later. However, for now it’ll get backlogged because it’s apparently important enough to have a one hour grooming meeting over but not important enough to get out for the next 13 service releases.
You’ll note that we made the expert decision to abstract the hideous complex functionality of std::cin and std::cout in the company’s common library. We’re currently calling into that library, but it’s dynamically linked via a build script that does not use Makefile in any capacity since we, as a team, wish to be aligned with a conference that was held in 2004 by someone that likely made their fortune patenting the Windows search feature.
Thus, we have a shell script linking everything together manually. Unfortunately, the build script seems to be failing and the deployment team refuses us to make changes to this file. Not really sure this compiles but we’ll probably find out later. I could try compiling it manually but… gosh it’s difficult navigating this virtual machine, the hell is the clock speed on this [CORPORATE APPROVED] CPU? 2 GHz? Well, at least we can be proud of the code, because we are all team players and we’re here to make a difference! Yeah!
Wait… what’s that? The PM redid the UI and updated the ACs? This story is outdated and needs to carry over to next sprint?
“These people are fucking stupid.”
Me
You think this is an exaggeration? Another strawman? I have burned my corneas reading line after line of shit code in so many codebases like this I can’t even begin to tell you how people ever thought this shit was a good idea. And then when I need to identify an issue, or try to optimize some garbage legacy code to make it somewhat understandable, half the time I have to work through a VM that has the specs of a 2002 Dell Latitude.
The real joke is they give you these machines that struggle to run the damn security software they bundle with it and then expect you to open a project with thousands of files in Visual Studio 2022. Nothing like running the most resource-intensive IDE ever made on a computer that would make history being the first device physically incapable of running Doom.
Being Blunt
The big problems of some open source and most proprietary software is the general lack of common sense. It’s a combo of every possible thing going wrong; businesses electing to make the worst decision they can for each scenario.
- Linters? Don’t need.
- Formatters? Don’t need.
- Consistent code? Don’t need.
- Performance? Optional.
- Build pipelines? Preferably unstable.
- Feature testing? Only after general availability for the feature.
- 1 hour meetings each day strictly for agile alignment? Required.
Does anyone see the problem above? I certainly do. Where’s the priority on the quality and architecture of the code? Where’s the focus on designing a maintainable application? Open source developers are working to make the best software and libraries that may be used across the world for a variety of projects. While never perfect, these projects at the least present the illusion of being well maintained.
Legacy software, specifically that existing within enterprise software, is full of problems because there’s less incentive to create the best thing possible and more incentive on the bottom line. Can we a.) get this feature out to customers and b.) make a profit off the feature development being done by the devs we pay to implement it? As long as these are satisfied, the business in general isn’t gonna care. No matter how much you whine.
I really like working on large teams and on enterprise applications because it feels like I’m making a difference, no matter how small it is. It’s satisfying working on a project where you see in the town halls “We’re #1 in these countries and in these spaces!” It’s cool to feel like the work I do matters. However, working on these business applications, as much as it fulfills me, is a complete pain in the ass for dumb reasons. It’s a shame that we have to deal with the sins of legacy code writers that based their entire identities on the words “encapsulation” and “abstraction”. But why bitch so much now? These are sins of the past. They may impact us, but what’s the deal? I’m bitching because these problems
CONTINUE. TO. HAPPEN.
Legacy code writers that added features 10 years ago still exist within the companies they’ve borderline crippled with their stupidity. Make no mistake, that person that ruined your Tuesday because they decided they had better things to do than… well… their job — these people STILL WORK. They are still active in the industry, they may even work at the very company you are at right now.
Even worse, the junior developers that learned under these “senior” engineers that were learning themselves have adopted the same practices. They think themselves paragons of knowledge and understanding when really they’ve only been taught how to peel and stick the stickers on a Rubik’s cube to solve it. Being newer to the field than my more senior colleagues, they’re less inclined to take my suggestions. As someone with less experience, my voice doesn’t matter as much. It’s annoying on its own, but when the very opposition to your words is in an effort to pollute the ecosystem that you are trying to thrive in? That’s when I have a problem.
This kind of disease festers in engineers who are desperate to meet deadlines and goals so they don’t risk working weekends or upsetting their superiors who are more than happy to lay them off and rehire one of the million other boot campers that wants to make six figures while still living in a musty basement. This kind of behavior is toxic. We are actively engaging in the same behavior that legacy writers did. What did we expect would happen when our feature blew up in our face on release?
So despite how stupid my examples may have been, despite how annoying my frustrations may appear, whether or not you think I pushed strawman after strawman to make a point, allow me to show some common disconnects I experience on a day-to-day when working with enterprise software.
| Expectation | Reality (in enterprise code) |
|---|---|
| Primary keys in relational DBs, in nearly all cases, should never be modified to preserve referential integrity. | Primary keys are just data. Who knows what part of our app touches it. |
| Interfaces should be used to abstract functionality across a variety of implementors. | Interfaces should be used for all types, including scalar types where possible (consider creating a class to wrap around the scalar, and a factory to create instances of said scalar wrapper). |
| Databases should be used to store data. | Databases can include anything to help the customer receive the feature on time. Source code being pushed as raw text to the DB is allowed. |
| Modern programming languages, while not perfect, have improved on the faults of various languages that came before them. | Story “J-29485 [SPIKE] Research Go to replace Express backend” was put into the backlog with comment “Out of scope”. |
| Making your application cross-platform/portable is beneficial. | The application should only run on Windows. |
| Proper documentation of code is critical to maintaining a large product. | Please only comment exactly what each line is doing, for every line, no matter how simple of self explanatory. Do not use “doc comments” that work with IntelliSense/LSPs. |
| Abstraction can be either simplification or obfuscation, and must be done correctly. | Each instance pointing to an API must be wrapped in a well-defined public interface that uses some interface or abstract class type to inherit from. It can only be created via a factory, which may also need a factory in turn. |
| Different programming patterns are useful and are not typically “stylistic” choices. They are chosen to best represent the intent of the code and to support future changes. | Use your favorite pattern to solve a problem. We highly suggest the builder pattern to instantiate an integer. |
The irony isn’t lost on me. I’ll probably be the same person that some poor dude complains about in the future, looking at my tasteful code that doesn’t take up half a damn screen to add two numbers together. Maybe there will be a dev renaissance where we embrace having a factory, two service handlers, an adapter interface that allows injections of various (1) adapters into the service handlers so the factory can generate a class that allows programmers to easily convert a string to an int.
These issues are what inspired me to start writing in a blog in the first place. The mounting frustrations with working with broken or poorly designed systems became almost too much to bear. These systems were not designed with dedication or care. These systems were designed by fools thinking they were geniuses, and now it’s our turn.
Software is much like a well built car. We’ll paint over the dirt, replace the panels, change the oil, maybe get an upgrade, but we’ll never fully be able to rid our beautiful car from the rot that follows and grows upon what we’ve left behind. No matter how hard we try, eventually, the car will rust. To an extent, it’s normal — cars break down, software fails, and life just isn’t always fair. What worries me is that instead of kind and benevolent leaders guiding us on how to carefully remove the rust, we have an industry built on being forced to spread it.