To RETURN or to not RETURN?

Posted by Patrick Tingen on 13-Jul-2017 07:28

Working my way through some OpenEdge courses, my eye fell on the explanation of the concept of procedures and how to return from them. One recommendation that struck me was this one:

A best practice is to place a return statement as the last statement of a procedure.

Now, nothing better than a religious war at the start of my holiday, so can anybody explain me why on earth I would want to have a RETURN statement at the end of my procedure? Like it doesn't return? Or does the AVM start hanging about or something?

There are only 2 cases where I would expect a RETURN statement:

1) If you get paid per line of code
2) If you are working with RETURN-VALUE

I think both of them are bad things, so why would you want to explicitely RETURN?

All Replies

Posted by Roger Blanchard on 13-Jul-2017 07:30

I believe it would reset the ERROR-STATUS

Posted by Laura Stern on 13-Jul-2017 07:31

I totally agree with you.  It makes no sense to me and is just a waste of time.

Posted by Brian K. Maher on 13-Jul-2017 07:44

The reason is that if you don’t do it any lower level RETURN “blah” will get propagated back.
 
Think a runs b runs c and only a return “test” statement in c.  If you message out return-value in a and b you will get “test”.
 
If you never use that optional value to the return statement then no need for this.

Posted by Roger Blanchard on 13-Jul-2017 07:56

ah...clears RETURN VALUE

Posted by Patrick Tingen on 13-Jul-2017 07:57

No, I don't use the RETURN-VALUE because I think it is a dangerous concept. It is not clear what it tries to do. If you really need to give something back to the caller, better make it explicit by using parameters. This also prevents you from accidentally breaking the system.

I have (more than once) broken a RETURN-VALUE concept, just by adding an extra intermediate call and not giving back the RETURN-VALUE. Simply because I overlook it. You cannot forget to use parameters if the program has them. Or at least the runtime will kindly remind you of them :)

Posted by Brian K. Maher on 13-Jul-2017 08:09

Hi Patrick,

I think it was one of those things that was added long, long ago but as you say there are much better options available now.

Brian

Posted by Laura Stern on 13-Jul-2017 08:23

Sorry - still don't agree.  If the procedure EVER does RETURN <value>, then it should always do so.  If it doesn't EVER do that, then it doesn't need a RETURN statement and that is part of the API of the procedure.  The caller should not be looking at RETURN-VALUE if the caller of the procedure never sets it.

Posted by Laura Stern on 13-Jul-2017 08:26

P.S. - The same comment regarding ERROR-STATUS. No one should be looking at ERROR-STATUS unless they just did a statement with NO-ERROR on it.  Otherwise its contents are unpredictable.  

Posted by Brian K. Maher on 13-Jul-2017 08:30

Hi Laura,
 
I think the recommendation exists to avoid the case where a -> b -> c and c does a return “blah”, b doesn’t care about it but doesn’t clear it and a uses it.  Bad code?  Absolutely.  I have seen stuff like this in support from older code that has been heavily modified over the years and there isn’t much doc on it.
 
Brian

Posted by Brian K. Maher on 13-Jul-2017 08:32

or checking ERROR-STAUS:ERROR when it is possible that a warning is returned.  :ERROR won’t see that but :NUM-MESSAGE > 0 will.

Posted by Peter Judge on 13-Jul-2017 08:32

I agree 100% that for (internal) procedures, RETURN is optional.  But I cannot stress strongly enough that it MUST be mandatory for methods and functions. (the compiler doesn’t require it).
 
I have been bitten hard by  the code below.  Note that the HandleGet has no RETURN statement. The code compiles and runs.
 
What do you think happens?
  1. Runtime error raised
  2. Method returns 0 / zero
  3. Method returns ? / unknown value
 
The answer is 3. Which is NOT what I thought, since integer default values are 0.   The moral here is that a clear RETURN statement makes it unambiguous what the value is.
 
   method protected integer HandleGet(input poRequest as IWebRequest):
    end method.
 
 
    method public integer HandleRequest( ):
        define variable webRequest as IWebRequest no-undo.
        define variable httpStatus as integer no-undo.
       
        assign webRequest = new WebRequest()       
               httpStatus = integer(StatusCodeEnum:None).
       
        /* Check http method and dispatch to corresponding handler */
        case MethodEnum:GetEnum(webRequest:Method):
            when MethodEnum:GET     then
                assign httpStatus = HandleGet(webRequest).
            otherwise                   
                
assign httpStatus = HandleNotImplemented(webRequest).
        end case.
       
        // Assume no malice here, just now knowing/forgetting to RETURN <int>.
        if httpStatus eq ? then
 
 

Posted by Marco Mendoza on 13-Jul-2017 08:34

just "END" or "END PROCEDURE"?

I always use end procedure, it takes more time but clarify the code.

Maybe  RETURN. END. is the equivalent version: hey! this is your end procedure line!

Posted by Peter Judge on 13-Jul-2017 08:36

Ain’t that the truth.
 
You should also not that the ERROR-STATUS (like the RETURN-VALUE) is *NOT* reset/changed automatically – it takes some action to do so.
 
So if you say
   DELETE OBEJCT objRef1 NO-ERROR.
 
And the object hasn’t been instantiated, the ERROR:ERROR-STATUS flag is TRUE until another error is raised  or you explicitly set it to FALSE.

Posted by Patrick Tingen on 13-Jul-2017 09:08

@Peter I agree that it is mandatory on functions and methods and if you think of it, it is kind of strange that the compiler does not raise a warning for you. Might be a good candidate for the -strict startup option

Posted by Peter Judge on 13-Jul-2017 09:22

Yep* - it’s a good rule to have.
 
 
 
 
* there’s a lot of muttering under my breath contained in that yup :)

Posted by Rick Terrell on 13-Jul-2017 10:04

Running another statement NO-ERROR also resets the ERROR-STATUS flag. 

Rick Terrell 
Principle Consultant, Professional Services 
Progress

Sent from my iPhone

Posted by James Palmer on 13-Jul-2017 10:58

I worked in a shop in the past where we had a global {&ResetError} preprocessor. All that did was assign something no-error to reset the flag. Horrible. It was everywhere in the code, just before any statement where the code was checking the value of error-status afterwards. A complete mess.

Posted by Lieven De Foor on 14-Jul-2017 04:51

Just stop using procedures altogether and only use classes/methods unless the language is forcing you to (e.g. bootstrapping, .p to access AppServer, async callback procedure etc...).

That seems to work for me ;-)

This thread is closed