Stored Procedure calls multiple stored procedures

Question

When calling several stored procedures for one stored procedure is this the right or best way to go about it on SQL Server 2008 ?

CREATE PROCEDURE [dbo].[DoStuff]
AS
BEGIN
    SET NOCOUNT ON;
    declare @result int
    BEGIN TRANSACTION
     BEGIN
      EXECUTE @result = dbo.UpdateTHIS @ID = 1   
       IF @result != 0
       ROLLBACK
      ELSE 
       EXECUTE @result = dbo.UpdateTHAT @ID = 21    
       IF @result != 0
        ROLLBACK
       ELSE
        EXECUTE @result = dbo.UpdateANOTEHR @ID = 15
        IF @result != 0
         ROLLBACK
        ELSE
         COMMIT
         SELECT @result 
     END    
END

Answer 1

IMHO, I don't think calling other stored procs from one stored proc is a very good idea. If there is an error, it would take a lot of man hours to figure out where the error happened. In some cases another developer may change a stored proc without telling anyone in the team. The number of things that can go wrong is too high.

It may be best to write all your sql in one big stored proc.

Edited: It also depends on how big your stored proc is going to be.

Answer 2

I highly recommend using TRY/CATCH blocks and RAISERROR instead of @@ERROR/@result checks. I have a blog entry that shows how to properly use transactions and TRY/CATCH blocks, including nested transactions to revert only the failed procedure call work so that the calee can resume a different path and continue the transaction, if it feels like: Exception Handling and Nested Transactions.

<Update>

You are being inconsistent with regard to procedures return mode. UpdateTHIS and UpdateTHAT return 0/1 as a return value, while the wrapper DoStuff returns as a result set (SELECT). It means you cannot write DoMoreStuff that calls DoStuff because it has to use INSERT ... EXEC to capture the result, and you'll quickly find out that INSERT ... EXEC cannot nest. I recommend using RETURN @result instead, for consistency.

</update>

I also have an unrelated commend, which is just an element of style: I find long IF... ELSE IF... ELSE IF ... ELSE IF ... blocks difficult to read and follow. I always found that expressing the same as a DO ... BREAK ... BREAK ... BREAK ... WHILE (FALSE) is easier do read. T-SQL does not have a DO ... WHILE construct, so a WHILE ... has to be used instead:

BEGIN TRANSACTION
WHILE (1=1)
BEGIN
  EXECUTE @result = dbo.UpdateTHIS @ID = 1;   
  IF @result != 0
  BEGIN
     ROLLBACK;
     BREAK;
  END 

  EXECUTE @result = dbo.UpdateTHAT @ID = 21    
  IF @result != 0
  BEGIN
     ROLLBACK;
     BREAK;
  END

  ...

  COMMIT;
  BREAK;
END

Again, this is no important as is just a code formatting style, but is a suggestion in case you agree that it results in code that is easier to read.

Answer 3

Nested ifs and not/nested ifs are different. Rollback will rollback a transaction, but keep going. I'd had some explicit RETURN commands to gurantee and make it obvious when and where you want the code to exit the stored procedure.