views:

42

answers:

2

Do you guys see any problems with the way I handle errors in the following stored procedure? I am still not confident enough with SQL to realize if I am doing anything wrong. Thank you :)

CREATE PROCEDURE [dbo].[UserAccounts_Create]
    @username varchar(255),
    @email varchar(255),
    @password nvarchar(127),
    @id bigint OUTPUT
AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;
    SET XACT_ABORT ON;

    DECLARE @now smalldatetime
    SET @now = GETDATE()

    DECLARE @userId bigint
    DECLARE @transactionSuccess bit
    SET @transactionSuccess = 0

    BEGIN TRANSACTION       
        INSERT INTO UserAccounts([username], [email], [password], [createDate], [lastLoginDate])
            VALUES (@username, @email, @password, @now, @now)

        SET @userId = SCOPE_IDENTITY()

        INSERT INTO Users([id], [firstName], [lastName])
            VALUES (@userId, '', '')

        SET @transactionSuccess = 1;

    COMMIT TRANSACTION

    IF (@transactionSuccess = 1)
        RETURN 0; -- Success.
    ELSE
        RETURN 1; -- Some unknown error occured.
END
+1  A: 

I would recomend you have a look at TRY...CATCH (Transact-SQL)

Using this you can BEGIN TRANSACTION and COMMIT TRANSACTION, or ROLLBACK in the event of an error.

astander
+3  A: 

If you hit an error that causes you to abort, RETURN 1; will never happen.

Instead of using that syntax, why not try out the new(ish)

BEGIN TRY
...
END TRY
BEGIN CATCH
...
END CATCH

syntax? It gives you much more control, and is (IMO) a bit easier to read.

Dave Markle

related questions