While I was maintaining one of our larger projects the other day, I came across something like this in an update trigger. I've anonymized the table and field names, but otherwise the logic is unaltered:

declare
  @id int,
  @val_old varchar(10),
  @val_new varchar(10)

declare ChangeCursor cursor for
select
  new.id,
  new.val,
  old.val
from deleted old
join inserted new on new.id = old.id

open ChangeCursor

fetch next from ChangeCursor
into @id, @val_new, @val_old

while @@fetch_status = 0
begin
  if ( @val_new <> @val_old )
  begin
    raiserror( 'The value may not be changed for id %d; old value: %s, new value: %s', 16, 1, @id, @val_old, @val_new )
    close ChangeCursor
    deallocate ChangeCursor
    return
  end

  fetch next from ChangeCursor
  into @id, @val_new, @val_old
end

close ChangeCursor
deallocate ChangeCursor

Quick, what does this code do? My eyes skipped over all the cursor management boilerplate and went straight for the text of the RAISERROR, which explained in a single sentence what took about 30 lines of SQL to accomplish. Now, before I get into my suggestions for how this could be rewritten to be both faster and more readable, I'd like to point out that this trigger doesn't make the common beginner mistake of assuming that the INSERTED and DELETED tables have only one row each. Triggers fire once per triggering statement, not once per row, and this code was written to handle that case. However, I still have a beef with this code, because it uses a cursor, which does two things that I don't like: perform row-based logic instead of set-based logic, and increase the ratio of boilerplate code to actual logic, which reduces readability.

Let's try rewriting this trigger using set-based logic and compare the results. All the above code does is loop through a result set until it finds a row where a certain field changed, uses the current row's values to report an error, and exits the trigger. If you're scratching your head wondering if simply raising an error in a trigger is enough to abort the operation, the answer is no. This particular trigger was written to be called by C# code that would open a transaction, execute the update, and then either commit or rollback depending on whether any exceptions occurred. Purely as an aside, I don't prefer this pattern; I prefer to keep my transactions in stored procedures. My opinion is that T-SQL is a data manipulation language and C# is a general purpose language, so when there is data manipulation to be done, the more specialized language should be the first pick over the general purpose one. But anyway, back to the rewrite! The rewritten code should:

• Detect whether any values changed in the value field
• Use the values of any row where values did change to report an error (no specific order was given in the cursor declaration)
• Exit the trigger if and only if the error condition occured

declare
  @id int,
  @val_new varchar(10),
  @val_old varchar(10)

select top 1
  @id = new.id,
  @val_new = new.val,
  @val_old = old.val
from deleted old
join inserted new on new.id = old.id
where new.val <> old.val

if ( @@rowcount > 0 )
begin
  raiserror( 'The value may not be changed for id %d; old value: %s, new value: %s', 16, 1, @id, @val_old, @val_new )
  return
end

This version is shorter mainly due to the removal of the boilerplate cursor code, but the result is not only just as fast or faster, but also much more quickly understood because the noise to logic ratio has been greatly reduced.