Bad form to have multiple return statements?

    General discussion

  • Starting on a stored procedure where I want to run about 4 or 5 validation checks ( set number) to verify some conditions before doing other statements (updates and inserts).  With each check I want to return a message string back to the user that indicates why that validation check failed.  Now, if all the validation passes, but there is a system error (caught in a catch block), that message should get returned instead.

    There are a few ways to handle this, but for some reason I started with something like this( with multiple blocks), returning immedately if a validation check didnt pass.

    set @somevar = select count field1 from table1 where...
    if (@somevar = 'X')
       set @message  = 'validation failed because...'
        return @message

    I think maybe a better approach is to set the various return messages as variables, then do a single select and handle the logic of which message to reurn to the user on the user side.

    In other words,

    select @validation_msg1, @validation_msg_2, @system_error_msg

    • Edited by shiftbit Thursday, July 11, 2013 1:06 AM
    Thursday, July 11, 2013 12:54 AM

All replies

  • As best practise records the validation in a ValidationLog table with contains MessagesId, MessageDate, BatchId, ErrorMessage etc with BatchId and select from that tables

    Thanks and Regards, Prajesh Please use Marked as Answer if my post solved your problem and use Vote As Helpful if a post was useful.

    Monday, July 22, 2013 1:21 PM
  • Early termination on validation errors is definitely a best practice. Otherwise it complicates your control flow.

    The right way to communicate errors to the caller is through THROW or RAISERROR. The only value you should RETURN from a stored procedure is 0 for success or -1 for failure.



    Monday, July 22, 2013 1:56 PM
  • Shouldn't validation be in the DDL in the form of CHECK() constraints and REFERENCES, so that the table never has bad data? Then you can scrub the data in the ETL layers of the system and not as part of some insert, or update operation at run time. 

    The bad news is that T-SQL doe not support the ANSI/ISO Standard CREATE ASSERTION statement yet. you can kludge around that with a WITH CHECK OPTION, but it is not pretty. 

    But to the original question: a module of code should have one and only one entry point, one and only none exit point and perform one and only one clearly defined task. Same rule in SQL as in any other language; remember freshman software engineering?

    I would go with the ANSI/ISO model for SQL/PSM. It is set-oriented; it is to collect a set of errors and assign them a severity. The user can then return a subset of the messages; some errors (security violation, connection failure,  etc) are show stoppers and we return immediately. Some can accumulate (possible overflow, etc). Some are warnings (NULLs dropped from aggregate function). 

    --CELKO-- Books in Celko Series for Morgan-Kaufmann Publishing: Analytics and OLAP in SQL / Data and Databases: Concepts in Practice Data / Measurements and Standards in SQL SQL for Smarties / SQL Programming Style / SQL Puzzles and Answers / Thinking in Sets / Trees and Hierarchies in SQL

    Monday, July 22, 2013 2:51 PM