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

good practice question

946279 Newbie
Currently Being Moderated
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 Expert
    Currently Being Moderated
    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 Journeyer
    Currently Being Moderated
    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 Newbie
    Currently Being Moderated
    Marwim, so you would opt for handling custom exceptions in such scenario, right?
  • 4. Re: good practice question
    946279 Newbie
    Currently Being Moderated
    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 Expert
    Currently Being Moderated
    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 Newbie
    Currently Being Moderated
    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 Guru Moderator
    Currently Being Moderated
    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 Guru
    Currently Being Moderated
    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 Journeyer
    Currently Being Moderated
    >

    >
    This is how lot of vendors milk there clients ;)
    lol
  • 10. Re: good practice question
    BillyVerreynne Oracle ACE
    Currently Being Moderated
    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 Journeyer
    Currently Being Moderated
    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 Journeyer
    Currently Being Moderated
    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 Journeyer
    Currently Being Moderated
    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 Expert
    Currently Being Moderated
    (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

Legend

  • Correct Answers - 10 points
  • Helpful Answers - 5 points