CodeSOD: Drop it Like it’s a Deployment
Zenith’s company went ahead on and outsourced 95% of their development to the lowest bidder. Said bidder promised a lot of XML and MVC and whatever TLAs sounded buzzwordy that day, and off they went. It’s okay, though, the custmoer isn’t just taking that code and deploying it- “Zenith” gets to do code reviews to ensure code quality. The general flow of the post-code-review conversation goes something like:
Zenith: This code shouldn’t go into production, hell, it’s so bad that a proud parent wouldn’t even hang it on their fridge.
Management: I’ll raise your concerns.
Outsourced Team: We did the needful, please review again.
Zenith: They didn’t change anything. It doesn’t even compile.
Offshore Team: There are too many barriers, we cannot hit deadlines, your team is too strict
Managment: Yeah… I guess you’re gonna have to lay off the contractors. Don’t be so strict in your code reviews. We have to deliver software!
The worst code ended up, not in the software, but in the deployment scripts. The team didn’t have and didn’t want a build environment (because they didn’t want to be expected to test their deployment scripts), so they essentially just guessed what the deployment scripts should be like and hoped for the best. They didn’t check them, they certainly didn’t run them.
For deploying changes to stored procedures, they got especially interested in using
DROP commands, like so:
DROP PROCEDURE [schema].[foo]; CREATE PROCEDURE [schema].[foo] AS…
DROP statements destroy the object and any grants associated with it- meaning the permissions got wiped out with every deployment. After a long weekend cleaning up a botched deployment, “Zenith” gave them a template to follow. All they needed to do was plug their code into a script that would never drop, but instead create/alter as needed.
They… “adapted” his script to their own processes.
IF EXISTS(select * from sys.all_objects where name = 'USPCandidateSearchElectionInfo') DROP PROCEDURE CF.USPCandidateSearchElectionInfo GO IF OBJECT_ID('[CFO].[USPCandidateSearchElectionInfo]') IS NULL BEGIN EXECUTE('CREATE PROCEDURE [CFO].[USPCandidateSearchElectionInfo] AS BEGIN SELECT NULL; END'); END GO ALTER PROCEDURE [CF].[USPCandidateSearchElectionInfo] @Request XML AS BEGIN --pages and pages of horrific code follow, the details of which are inconsequential RETURN @@ROWCOUNT END
Not only did they keep the
DROP, thus defeating the entire reason why he had given them a script in the first place, they also couldn’t even get so far using the same name forr the procedure all the way through.
“Zenith” raised this with management, and was once again scolded: “Code reviews are supposed to facilitate development, not provide a barrier to deployments.”
is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies – only $9.95 shipped!