1 2 3 Previous Next 33 Replies Latest reply: Feb 5, 2013 5:02 AM by Bawer RSS

    good practice question

    946279
      there is a procedure p(i in number, o out number) that checks some data and finishes this way if some criteria are met:
      procedure p(i in number, o out number) is
      ...
      select count(*) into v_counter
      from ... ;
      
      if v_counter > 0 then
        o := 123;
        return;
      end if;
      ...
      exception
        when others then
        o := 99999;
      end;
      is this a good practice to finish this way (ie. return) or some other ways would be better in terms of "good practice" (like raising custom exception and handling it in exception block where out variable could be set up)?

      I would appreciate your suggestions about good practices in scenario above.

      thank you
        • 1. Re: good practice question
          Marwim
          Hello,

          controling exceptions with out-parameters is "bad". You have to trust that every caller will check the return code. If he does not, then noone will notice that the process went wrong. When you rise an exception in case of an error, then the error will be propagated to the caller and he must handle ist, at least it cannot get unnoticed.

          Regards
          Marcus
          • 2. Re: good practice question
            Bawer
            943276 wrote:
            exception
            when others then
            o := 99999;
            end;
            How you you want to know the error which was happened (don't you need more info in case of errors) ?
            And don't you want to return this error back?
            • 3. Re: good practice question
              946279
              Marwim, so you would opt for handling custom exceptions in such scenario, right?
              • 4. Re: good practice question
                946279
                yes Bawer, this was my concern too. thanks!

                do you think this would be better?
                procedure p(i in number, o out number) is
                  custom_ex exception;
                  pragma exception_init(custom_ex, -20999);
                  ...
                begin
                  ...
                  select count(*) into v_counter
                  from ... ;
                 
                  if v_counter > 0 then
                    raise custom_ex;
                  end if;
                  ...
                exception
                  when custom_ex then
                    o := 123;
                  when others then
                    o := sqlcode;
                end;
                Edited by: 943276 on 2013-02-04 10:27
                • 5. Re: good practice question
                  Marwim
                  so you would opt for handling custom exceptions in such scenario, right?
                  There are 2 kinds of errors:
                  * Those that you know might happen and where you can continue the program after handling it (e.g. providing a default value for a variable in case of a NO_DATA_FOUND).
                  * And those that are unexpected or that require the process to cancel the current transaction.

                  The first will be handled and no exception is propagated to the caller. The second will go unhandled to the caller (or logged followed by a RAISE) and the calling procedure has to handle it or propagate it again to its caller.

                  In most cases an exception is best kept unhandled up to the topmost calling procedure, where it will be handled, maybe to rollback the transaction and to give a meaningful error message to the user.

                  Regards
                  Marcus
                  • 6. Re: good practice question
                    610606
                    At least every answer to that is a personal opinion ;-)
                    Here's mine:

                    Raising a custom exception to end the procedure when nothing bad happened is a hidden GOTO - very bad style.
                    Using a return in the middle of the procedure is a hidden GOTO. Better than an exception, so only bad style.

                    The best way imho is to reverse the if-condition, so the procedure ends at bottom:
                    ----------
                    procedure p(i in number, o out number) is
                    ...
                    select count(*) into v_counter
                    from ... ;

                    if v_counter = 0 then
                    <do something useful>
                    else -- v_counter > 0
                    o := 123;
                    end if;
                    exception
                    when others then
                    o := 99999;
                    end;
                    ----------

                    Regards
                    6502
                    • 7. Re: good practice question
                      BluShadow
                      6502 wrote:
                      At least every answer to that is a personal opinion ;-)
                      Here's mine:

                      Raising a custom exception to end the procedure when nothing bad happened is a hidden GOTO - very bad style.
                      Not particularly bad at all.

                      An "exception" doesn't have to mean that something "bad" has happened. It means that something different to the "expected norm" has happened. It's perfectly acceptable to use user defined exceptions in a program unit to prevent further processing taking place of that unit.
                      Yes, people do unfortunately use exceptions for the wrong reasons or often use the WHEN OTHERS exception when they shouldn't, but that doesn't mean that using exception handling, if done correctly, is a bad style.

                      Using GOTO statements on the other hand... that's what I'd consider bad style, and is a poor legacy from the BASIC language.
                      • 8. Re: good practice question
                        Karthick_Arp
                        There are situations where as a vendor you are required not to show the exact error message to the client. When a unhanded error occurs you might want to give a generic error message saying something like "System Error Contact System Administrator".

                        But in that case the vendor has to log the exact error message in a seperate place internally for his further reference. Such a thing can be achieved using tools like DBMS_UTILITY.FORMAT_ERROR_BACKTRACK, DBMS_UTILITY.FORMAT_ERROR_STACK and DBMS_UTILITY.FORMAT_CALL_STACK.

                        This is how lot of vendors milk there clients ;)
                        • 9. Re: good practice question
                          Rahul_India
                          >

                          >
                          This is how lot of vendors milk there clients ;)
                          lol
                          • 10. Re: good practice question
                            Billy~Verreynne
                            943276 wrote:
                            there is a procedure p(i in number, o out number) that checks some data and finishes this way if some criteria are met:
                            ..snipped..
                            is this a good practice to finish this way (ie. return) or some other ways would be better in terms of "good practice" (like raising custom exception and handling it in exception block where out variable could be set up)?
                            I would call using out params as exception/error codes wrong. Never mind good or acceptable practise. It is just plain wrong. As wrong in PL/SQL as it would be wrong in Java, C#, Pascal or another language that implements exceptions.

                            Why? Because in such a language an exception is raised to indicate an error condition - an exception to the norm has occurred. If code suppresses the exception, that code is now saying that there is no error condition. It explicitly tells the caller that processing was successful and no exceptions to the norm have occurred.

                            Returning the error code as an out parameter is not acceptable - as the "contract" in such a language, between the caller and that code, states that errors are handled as exceptions. That is how the language is designed to be used. That is how the run-time of the language works.

                            Violating this fundamental concept in such a language makes absolutely no sense.
                            • 11. Re: good practice question
                              Bawer
                              943276 wrote:
                              yes Bawer, this was my concern too. thanks!

                              do you think this would be better?
                              No, It is still bad and I can't know what is your business requirements. Basically, you should avoid to handle the system exceptions in sub procedures/functions. You can handle the user defined exceptions but you should think again, because with user defined exceptions, you must:
                              1) check the error
                              2) raise error (raising, really needed?? )
                              3) catch error in other units
                              4) do error process (rollback or logging)

                              instead all of this you can keep it simply

                              1) check the error
                              2)no error:continue or commit /error:rollback or logging

                              Use a function if possible, which checks errors instead of raising user defined exceptions and than handling again.
                              • 12. Re: good practice question
                                Bawer
                                Marwim wrote:

                                There are 2 kinds of errors:
                                * Those that you know might happen and where you can continue the program after handling it (e.g. providing a default value for a variable in case of a NO_DATA_FOUND).
                                select count(*) ...
                                won't generate a NO_DATA_FOUND exception. In case, there is no data, it will return 0 back.
                                • 13. Re: good practice question
                                  Rahul_India
                                  Bawer wrote:
                                  943276 wrote:
                                  instead all of this you can keep it simply

                                  1) check the error
                                  2)no error:continue or commit /error:rollback or logging

                                  Use a function if possible, which checks errors instead of raising user defined exceptions and than handling again.
                                  Can you just give a small example?
                                  • 14. Re: good practice question
                                    Marwim
                                    (e.g. providing a default value for a variable in case of a NO_DATA_FOUND).
                                    select count(*) ...
                                    won't generate a NO_DATA_FOUND exception. In case, there is no data, it will return 0 back.
                                    With select count(*) you can check the existence, but not get the actual value.
                                    When you want to select a value into a variable and there is no current entry in the db, then you can catch the NO_DATA_FOUND and then give the variable a default value and continue with your process.
                                    1 2 3 Previous Next