Years ago, Karthika was working as a contractor. She got hired on to develop an intranet application for a small government project. The website in question was already in use, and was absolutely mission critical for the organization, but it also had a very small user base- roughly five users depended on this application.
When Karthika started, she immediately encountered a few surprises. The first was the size of the team- 8 developers, including a Team Lead. That seemed like a large team for that small number of users, and that didn't even include the management overhead. The code base itself was similarly oversized; while the product was important, it was a pretty prosaic CRUD app with a few tricky financial calculations involved.
The core surprise was how awful the application was to use. It was slow to the point of sometimes timing out in the browser, even when running on your local machine. It was buggy, and even when Karthika patched those bugs, there was so much duplicated code in the system that the same bug would linger hidden in other screens for months. "I thought you said you'd fixed this," was a common refrain from the users.
This was long enough ago that the UI was built in ASP.Net WebForms, but new enough that the data access was handled by Entity Framework. And it was one specific feature of WebForms that they were abusing that made everything terrible: UserControls.
UserControls were designed to let developers create reusable widgets. For example a "User Information" screen may group the "User Name" and "Password" fields into a single "Credentials" UserControl, while the address fields would all get grouped together in an "Address" UserControl. That same "Credentials" control could then be dropped into other pages.
When the user interacts with this data, Entity Framework can lookup a User object, hand it off to the UserControls, who allow the user to manipulate it, and then the controls can invoke the save on the User.
The Tech Lead had encountered a problem with this. You see, he didn't want to share the same reference across controls because of "separation of concerns". So instead, each UserControl would create its own User object, populate it with database values, and then let the user interact with it. This meant when each UserControl had its own picture of the user object, and when it was time to save the data on the page, one control could overwrite the changes made by another control.
So the Tech Lead invented CopyOldValues
, a method which, during a save operation, would go out to the database, fetch the current data, and then compare it to the object being saved. Any NULL values in the object being saved would be updated to the database values, and then the object would be saved. This way, a UserControl could have a whole User object, but only populate the fields it was responsible for, leaving the rest as null. So yes, this meant that to save an object to the database, it required two round-trips to the database, per UserControl. And each page could have multiple UserControls.
Karthika saw this, and put together a simple plan to fix this problem: just use the frameworks like they were meant to be used and cut this whole CopyOldValues
nonsense out. She went to the Tech Lead and laid out a plan.
"This isn't an issue," he said. "You're wrong to be worrying about this. Stop wasting my time, and stop wasting yours. Instead, you should look into the date bug."
So, Karthika tracked down the issue related to the date bug. Specifically, the database and the application were supposed to allow certain date fields to be NULL. But, since CopyOldValues
used NULLs to decide which data to save, it was impossible to update a stored value to a NULL. Once again, the fix was obvious: just stop doing this weird intermediate step.
"Wrong," the Team Lead said. "That's totally not the correct way to do it. I have a better plan already."
The "better plan" was to create a custom save method for each UserControl- of which there were hundreds. Each one of these would define an array which used the string names of each field it was responsible for, and then the object and the array would get passed to a new method, FindDifferences
, which would use reflection to inspect the object, copy the updated values to a new object, and prepare to save to the database.
The shocking end result of this, however, is that this made the application even slower. It didn't reduce the number of database round trips, and it added this whole reflection step which made accessing properties even slower. Despite only having five users, and running on a decently powerful machine, it was nigh unusuable. The Team Lead knew what the problem was though: the machine wasn't powerful enough.
Strangely, however, throwing hardware at the problem didn't fix it. So the Team Lead invented his own caching solution, which made things worse. He started reinventing more wheels of Entity Framework and made things worse. He started copy/pasting utility functions into the places they were used to "reduce overhead", which didn't make things worse but made every developer's life demonstrably worse as the repeated code just multiplied.
These problems made the customer angry, and that anger eventually turned into an all hands meeting, with representatives from the client side and the project manager as well. After the venting and complaining was over, the project manager wanted explanations.
"Why," she said, "aren't we able to fix this?"
A round of blamestorming followed, but eventually, Karthika had to get specific and concrete: "We have a set of fixes that could address many of these problems, but the Tech Lead isn't letting us implement them and isn't giving us a good reason why we can't."
The project manager blinked, puzzled. "Who? There's no defined tech lead on this project. You're a team of peers."
"Well," the 'Tech Lead' said, "I… uh… have seniority."
"Seniority?" the project manager asked, no less confused. "You started two weeks earlier, and that was just because you were the one contractor on the bench and we needed someone to knowledge-transfer from the outgoing team."
The Project Manager had been overwhelmed by handling customer complaints, and hadn't been able to carve out time to attend any of the development team meetings. This meant that the Tech Lead's self-appointed promotion went unnoticed for eight months. At this point, the project was too far off the rails for any hope of recovery. The government office "fired" the contracting firm the next week, and all the developers, including Karthika, were fired from the contracting firm the week after that.