CodeSOD: Delete This

About three years ago, Consuela inherited a giant .NET project. It was... not good. To communicate how not good" it was, Consuela had a lot of possible submissions. Sending the worst code might be the obvious choice, but it wouldn't give a good sense of just how bad the whole thing was, so they opted instead to find something that could roughly be called the median" quality.

This is a stored procedure that is roughly about the median sample of the overall code. Half of it is better, but half of it gets much, much worse.

CREATE proc [dbo].[usermgt_DeleteUser] ( @ssoid uniqueidentifier )AS begin declare @username nvarchar(64) select @username = Username from Users where SSOID = @ssoid if (not exists(select * from ssodata where ssoid = @ssoid)) begin insert into ssodata (SSOID, UserName, email, givenName, sn) values (@ssoid, @username, 'Email@email.email', 'Firstname', 'Lastname') delete from ssodata where ssoid = @ssoid end else begin RAISERROR ('This user still exists in sso', 10, 1) end

Let's talk a little bit about names. As you can see, they're using an internal" schema naming convention- usermgt clearly is defining a role for a whole class of stored procedures. Already, that's annoying, but what does this procedure promise to do? DeleteUser.

But what exactly does it do?

Well, first, it checks to see if the user exists. If the user does exist... it raises an error? That's an odd choice for deleting. But what does it do if the user doesn't exist?

It creates a user with that ID, then deletes it.

Not only is this method terribly misnamed, it also seems to be utterly useless. At best, I think they're trying to route around some trigger nonsense, where certain things happen ON INSERT and then different things happen ON DELETE. That'd be a WTF on its own, but that's possibly giving this more credit than it deserves, because that assumes there's a reason why the code is this way.

Consuela adds a promise, which hopefully means some follow-ups:

If you had access to the complete codebase, you would not EVER run out of new material for codesod. It's basically a huge collection of How Not To" on all possible layers, from single lines of code up to the complete architecture itself.

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.