Is it advisable to call Function directly in Case statement?

Jeevan Anand

Pseudo code of my actual code:

create function CALCULATE_nu ( a number, B number )

return number as

c number;

begin

C := a+ B;

return c;

end;

/

set serveroutput on;

begin

  case when CALCULATE_NU (5,7) > 10 then

    DBMS_OUTPUT.PUT_LINE('Number is greater than 10');

  else

    DBMS_OUTPUT.PUT_LINE('Number is less than 10');

  end case;

end;

/

During Code review, my reviewer asked me to store the Function- return value in varaible and validate by if or case statement. Can anyone tell me the difference between calling directly in Case statement or proposed method different?

Is there any issue in calling Function in case statement like above code?

BEDE

If the function is not deterministic and it is called several times with the very same parameters, then it's best to store the return value in a variable in order to avoid computing again and again the same thing.

On of the most monumental piece of code I encountered which shows how not to use functions was something like below:

if f(x,y,z)=1 then

...

elsif f(x,y,z)=2 then

...

else

...

end if;

So, the same function was computed twice...

And that piece of code was in Oracle Forms and the function, as far as I recall, was also inside the form and contained some SQL, so that there was also the roundtrip to the database...

Still, if in a case a function is called with various parameters (not with the same parameters several times!), then there is no performance improvement if you store that value in some variable and you just have to bother with declaring that variable which you will not use further on and will just take some memory.

Billy Verreynne

Typically you will use a function as a getter - to get a value. Validation rules can then be applied, prior to using the value returned by the function.

In such a case, your code will have multiple references in code statements, to the function value.

It is far better to store the function value in a local variable, and have multiple references to this variable, than have multiple expensive references to the function (running the same function code, returning the same value, numerous times).

mathguy

I don't see an issue, and with my temperament (which is not necessarily the same as anyone else's) I know I would push hard against this requirement. To me it makes no sense.

A variable to store the return value of the function would make sense if you used the value more than once in your code. You do not.

Someone will say "but what if you need to change the code in the future, you will be glad you made up a variable and used it." My view on that is "we'll cross that bridge when we get there." No one can guess correctly all the changes the code will have to go through in the future, and trying to anticipate all such hypothetical events is a fool's errand. And the amount of work is the same whether you do it now or at that time. I don't see how doing it now, in anticipation of some speculative change in the future, will save time overall for anyone.

I do, on the other hand, see the merit of writing the code exactly as you did. If you created a variable in a declarations section, then assigned a value to it at the top of your execution block, and then used the variable 150 lines of code later, you would have to do a search for the variable name to remember what it stood for. Even more so for someone who didn't write the code, but will be tasked to maintain it in the future. No such complication when you call the function directly where you use its return value - the function call is right there for all to see.

Of course, in many cases people will not push back; it takes less effort to just do as you are told and move on to something else. In my opinion that's a shame; that's how we end up with all sorts of well-established but nonsensical customs, rules and policies.

mathguy

... For that matter, I wouldn't write the function code the way you did. Rather, and in the same spirit:

create function calculate_nu ( a number, b number )return number asbegin  return a + b;end;/

There is no need for the local variable c here.

Jonathan Lewis

I'd agree with everything that mathguy wrote.

Couple of extra points, though:

a) While the response might be appropriate to your simplified code, there might be sufficient extra complexity in the production code to make people answer differently.

b) you could even write the code fragment in a way that makes the "got to create a variable" argument even weaker:

begin

        dbms_output.put_line(

                case when calculate_nu (5,7) > 10

                        then 'Number is greater than 10'

                else

                        'Number is less than 10'

                end

        );

end;

/

c) mathguy's argument about "the variable declaration could be a long way from the usage" can be answered easily because you can declare an entire pl/sql block pretty much wherever you want, so the following could be embedded almost anywhere in the code (this doesn't mean I'd want to do it, but if I had to enlarge a fragment from (b) above top allow for multiple cases that's the way I might do it):

declare

        m_dummy number := calculate_nu (5,7);

begin

        dbms_output.put_line(

                case when m_dummy > 10

                        then 'Number is greater than 10'

                else

                        'Number is less than 10'

                end

        );

end;

/

It's probably worth pointing out that for the specific "two options only " case of (c) the PL/SQL optimizer might be smart enough to optimize out the declaration and effectively turn the compiled code into the (b) form that your supervisor wasn't keen on.

Regards

Jonathan Lewis

L. Fernigrini

I agree with all the comments.

Since you just posted a pseudo-code version, surely simplified, I just wanted to mention that on some cases I would define the variable and assign it the value of the function if the function call itself is pretty long, in order to keep readability.

I sometimes prefer having something like this:

vPrice := fnGetPrice ( vProductId, vCustomerID, vInvoiceDate, vDiscountListCode, vPaymentMean, vCurrency);

CASE WHEN vPrice...

Rather than:

CASE WHEN fnGetPrice ( vProductId, vCustomerID, vInvoiceDate, vDiscountListCode, vPaymentMean, vCurrency) ....