CodeSOD: The Last of His Tribe

"The best thing about working here," Jaeson was told by his boss Renee on the walkthrough of the dev area on his first day, "is that we work closely with the customers, meaning you never EVER have to cash checks the salespeople wrote: you're only held to what your team promised." This sounded good to Jaeson, who was considerably more concerned about getting away from his previous company's open floor plan than the possibility of customer interaction, but hey, he'd take the icing on the cake. It sounded like he was in for a very productive time.

"Let's see, where to put you," mused Renee, glancing over the pods of cubicles. "Aha, I think Zebra Tribe will work." Zebra Tribe, it turned out, was the name of a scrum team; Jaeson would be working closely with the other members of that tribe, as they were called, in the coming months in order to accomplish tasks during sprints.

Over the next three months of working in Zebra Tribe, Jaeson often thought back to that original introduction and laughed. True, there were no salespeople involved with the customers; however, in order to keep customer happy, engineers themselves were forced to promise things they knew wouldn't get done in the arbitrary time-box of a sprint. They were forbidden from explaining Scrum, as it "confused people", so instead had to speak in terms of releases, and nobody wanted to hear "It won't be in the next release, and we're not sure when it'll get done" as an answer. So they were forced to cram more and more work into sprints, leading to rushed work, mountains of technical debt, and no documentation. Even having a fifth man instead of a usual four-person tribe didn't help Zebra much.

"How are you settling in?" Renee asked, at the first so-called biweekly one-on-one three months in. She listened intently to his criticisms of the process before smiling and changing the subject entirely: "Well I've got good news for you. I'm moving you to Elephant Tribe after the current sprint."

Elephant Tribe? thought Jaeson rapidly. I've never heard of Elephant Tribe. Hyena, Mongoose, and Snake, but Elephant? It wasn't a mystery for long; instead of a pod of cubicles like the other teams, Elephant Tribe had a single cube in the corner, where there was some space left over. In this lonely cube sat Bill, the Last of his Tribe, who was overjoyed to be given more resources to work with.

Jaesan was given a second cube in a different corner; gone were the days of active collaboration and late-night nerf wars to keep morale up. He was also given a starter project to get up to speed on the ancient product that Elephant maintained. "We don't use scrum really," explained Bill. "Not much point with just me. Maybe it'll be different soon, though. I hear they're gearing up for a refresh project, keep the UI up to date. Good thing, too; I mostly do support these days. Hasn't been a real project on this thing in, oh... two years? Three?"

Jaesan plunged into the task, getting familiar with the codebase. It was a brutal chore, but he managed to get the basic feature working with hardcoded parameters. Then he opened the configuration class.

public class SplineReticulationParams { public final bool DEFAULT_RETICULATE_SPLINES_ASYNCHRONOUSLY = true; public final int DEFAULT_SPLINE_RETICULATION_MAX_THREADS = 10; // The following fields are deprecated, but cannot be removed without breaking backwards compatibility with older config files. public final int DEFAULT_MIN_FOOBAR_SIZE = 1; // Deprecated public final int DEFAULT MAX_FOOBAR_SIZE = 10; // Deprecated public final bool DEFAULT_RETICULATE_SPLINES_USING_FOOBAR = false; // Deprecated public final bool DEFAULT_SEND_SPLINES_TO_REMOTE_SERVER_FOR_RETICULATION = true; public final int DEFAULT_MAX_SPLINES_PER_REMOTE_REQUEST = 100; public final String DEFAULT_REMOTE_SERVER_NAME = null; // [Submitter's note] Yes, null was used as the default several times. public final bool reticulateSplinesAsynchronously; public final int splineReticulationMaxThreads; // The following fields are deprecated, but cannot be removed without breaking backwards compatibility with older config files. public final int minFoobarSize; // Deprecated public final int maxFoobarSize; // Deprecated public final bool reticulateSplinesUsingFoobar; // Deprecated public final bool sendSplinesToRemoteServerForReticulation; public final int maxSplinesPerRemoteRequest; public final String remoteServerName; // ... snip ... public SplineReticulationParams( bool reticulateSplinesAsynchronously, int splineReticulationMaxThreads, int minFoobarSize, int maxFoobarSize, bool reticulateSplinesUsingFoobar, bool sendSplinesToRemoteServerForReticulation, int maxSplinesPerRemoteRequest, String remoteServerName, // ... snip ... ) { this.reticulateSplinesUsingFoobar = reticulateSplinesUsingFoobar; this.reticulateSplinesAsynchronously = reticulateSplinesAsynchronously; this.splineReticulationMaxThreads = splineReticulationMaxThreads; this.remoteServerName = remoteServerName; this.minFoobarSize = minFoobarSize; this.maxFoobarSize = maxFoobarSize; this.sendSplinesToRemoteServerForReticulation = sendSplinesToRemoteServerForReticulation; this.maxSplinesPerRemoteRequest = maxSplinesPerRemoteRequest; // ... snip ... } public SplineReticulationParams( bool reticulateSplinesAsynchronously, int splineReticulationMaxThreads) { SplineReticulationParams( reticulateSplinesAsynchronously, splineReticulationMaxThreads, DEFAULT_MIN_FOOBAR_SIZE, DEFAULT MAX_FOOBAR_SIZE, DEFAULT_RETICULATE_SPLINES_USING_FOOBAR, DEFAULT_SEND_SPLINES_TO_REMOTE_SERVER_FOR_RETICULATION, DEFAULT_MAX_SPLINES_PER_REMOTE_REQUEST, DEFAULT_REMOTE_SERVER_NAME // ... snip ... ); } public SplineReticulationParams( bool reticulateSplinesAsynchronously, int splineReticulationMaxThreads, bool sendSplinesToRemoteServerForReticulation, int maxSplinesPerRemoteRequest, String remoteServerName) { SplineReticulationParams( reticulateSplinesAsynchronously, splineReticulationMaxThreads, DEFAULT_MIN_FOOBAR_SIZE, DEFAULT MAX_FOOBAR_SIZE, DEFAULT_RETICULATE_SPLINES_USING_FOOBAR, sendSplinesToRemoteServerForReticulation, maxSplinesPerRemoteRequest, remoteServerName, // ... snip ... ); } // ... snip ...}

Not only did he have to add the fields, he also had to add a new constructor ,and then an old constructor that calls the new one with some sensible defaults in case people didn't care about his new feature. That all seemed to work, so he adjusted the config to use a different value than the default... and everything broke. What? He skimmed further down the config class and found the following:

// [Submitter's note] Found wherever the params had to be copied (e.g. to override a single param):SplineReticulationParams paramsCopy = SplineReticulationParams( params.reticulateSplinesAsynchronously, params.splineReticulationMaxThreads, params.minFoobarSize, params.maxFoobarSize, params.reticulateSplinesUsingFoobar, params.sendSplinesToRemoteServerForReticulation, params.maxSplinesPerRemoteRequest, overrideRemoteServerName, // ... snip ... );

Despite there being no copy constructor, when several generations of previous programmer had needed to override some parameters, they had made a fresh copy. After all, there were no setters for them to update. In desperation, he took a walk over to Bill's desk to stretch his legs.

"Hey Bill, why didn't we use a Protocol Buffer for our param class?" he asked, innocently.

"A what?"

"...nevermind, I think I figured out why."

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!