progress 11.3.1
I was writing some code using test-driven development, and was trying to fix a failing test, when the test in question refused to pass, as the returned result was always "".
What basically is happening is that the return for a longchar is always blank given this code. It's fine for other datatypes such as char, date etc.
I presume that this is a bug - could someone test and check it on different versions of progress ?
have a look at this class (it's much simplified, but still shows the error)
class temp.foo:
method public char notfoo():
def var lv_data as char no-undo.
assign lv_data = "data".
return lv_data.
finally:
return lv_data.
end finally.
end method.
method public longchar bar():
def var lv_data as longchar no-undo.
assign lv_data = "foobar".
return lv_data.
finally:
return lv_data.
end finally.
end method.
end class.
now run this procedure
def var a as temp.foo no-undo.
a = new temp.foo().
message string(a:notfoo()) view-as alert-box.
message string(a:bar()) view-as alert-box.
guess you get 'data' and 'empty' so either way it's inconsistent behaviour (aka, might be a bug) but imho you should resist the temptation to use finally for anything else beside clean-up... it there is an error that you need to catch then return from catch block else let it go wild :)
It’s caused by the finally block. Interesting. Why do you return values from the FINALLY block???
If you return from the actual code only, you get the value. Not sure if it’s bug or feature.
11.3.2.
yeah, it's the inconsistency that gets me. However, I believe that it should always return a value, wether it's nice looking code or not ;)
Mike, I was returning a value from the finally block because I was wanting to always return the value of lv_data at any point in time. Ans was being lazy and didn't want to have to remember to put checks everywhere.
However if you replace the return lv_data on lines 7 and 20 with just return then it also works just fine.
This is actually how I hit the error. I didn't like seeing the little warning symbol in the editor next to "return."
this warning message actually says
"WARNING: RETURN statement in user defined function or method 'notfoo' is missing a return value expression. At runtime it will return the unknown value. (2750)"
which is kind of ironic as it actually makes the whole thing work ;)
If you always want to return something, then you should remove the first RETURN statement, and only use the one in the FINALLY block.
don't take the above code as an example of the actual code. In this case there was a choice right at the start (it's a string parsing routine, if there is no "\" in the string then there's no need to parse, hence it makes sense to return straight away.
Otherwise I would have to reverse the test
instead of
/** no need to parse if \ is not present as there are no escapes */
if index(lv_Data,"\") eq 0 then return lv_Data.
I would have to say
/** no need to parse if \ is not present as there are no escapes */
if index(lv_Data,"\") gt 0 then do: /* load of code */
and have to indent the whole block of code in the do .. end.
I prefer the first way ;)
Slightly off topic, but just a small note. I would be mindful of the \ if you ever needed to compile the code in Unix environments. Always remember to protect your backslashes with a tilde to avoid compilation issues.
Currenty:
/** no need to parse if \ is not present as there are no escapes */
if index(lv_Data,"\") eq 0 then return lv_Data.
/** no need to parse if \ is not present as there are no escapes */
if index(lv_Data,"~\") eq 0 then return lv_Data.
of course. thanks for the reminder ;)
On 2 April 2014 10:42, Henri Christian Koster
wrote:
> RE: bug with longchar
> Reply by Henri Christian Koster
>
> Slightly off topic, but just a small note. I would be mindful of the \ if
> you ever needed to compile the code in Unix environments. Always remember to
> protect your backslashes with a tilde to avoid compilation issues.
>
>
>
> Currenty:
>
> /** no need to parse if \ is not present as there are no escapes */
> if index(lv_Data,"\") eq 0 then return lv_Data.
>
> Preferred:
>
> /** no need to parse if \ is not present as there are no escapes */
> if index(lv_Data,"~\") eq 0 then return lv_Data.
>
> Stop receiving emails on this subject.
>
> Flag this post as spam/abuse.
--
Julian Lyndon-Smith
IT Director,
dot.r
http://www.dotr.com
"The bitterness of poor quality remains long after the sweetness of
low price is forgotten"
Follow dot.r on http://twitter.com/DotRlimited
I still don’t see why you need 2 RETURN’s for the value.
When the code comes to the end, you return the value – there is no need to rely on the FINALLY block RETURN.
When the code errors out first, the returned value is irrelevant anyway.
Good point J
I recommend always compiling any code that needs to run on UNIX on a Jenkins/ANT/PCT system after each source commit .
Hey Mike. see my response to Lieven for the reason.
meh. ok , just for you ;)
method public longchar parseString(p_string as char):
def var lv_data as longchar no-undo.
def var lv_i as int no-undo.
fix-codepage(lv_Data) = "UTF-8":u.
assign lv_data = p_string.
/** no need to parse if \ is not present as there are no escapes */
if index(lv_Data,"\") gt 0 then
do:
assign lv_data = replace(lv_Data,'~\"','"')
lv_data = replace(lv_Data,'~\~\','\')
lv_data = replace(lv_Data,'~\/','/')
lv_data = replace(lv_Data,'~\b','~b')
lv_data = replace(lv_Data,'~\f','~f')
lv_data = replace(lv_Data,'~\n','~n')
lv_data = replace(lv_Data,'~\r','~r')
lv_data = replace(lv_Data,'~\t','~t')
lv_i = index(lv_Data,"\u").
/** handle unicode string */
do while lv_i gt 0:
assign lv_unicode = substr(lv_data,lv_i + 2,4).
lv_i = index(lv_Data,"\u").
/** more unicode handling */
end.
/** more parsing stuff */
/** etc etc etc */
end.
return lv_data.
end method.
so now I need to have a needless do .. end block with the indentation that comes with it. looks messy.
and the whole point is that the return in the finally .. end finally block works for all other datatypes except longchar
Thanks - next time, please add an include file and UPPERCASE the code … J
And under which case would the RETURN lb_data just before the END METHOD not return be enough? i.o.w. a RETURN in a FINALLY block make any sense?
>>Thanks - next time, please add an include file and UPPERCASE the code … J
lmao
the case would be that if there was no "\" in the string to be parsed, rather than the do : end block I could just
if index(lv_Data,"\") eq 0 then return lv_data.
right at the start, and not have to have the do .. end block at all
as mentioned, I could also just do
if index(lv_Data,"\") eq 0 then return.
which actually works - but gives a little warning sign in appbuilder (which I also don't like ;) )
To me using a RETURN with no value and relying on the RETURN lv_data in the FINALLY to return the value is a funny way of coding. How about a LEAVE in the first place?
>>To me using a RETURN with no value and relying on the RETURN lv_data in the FINALLY to return the value is a funny way of coding.
I agree ;) That's why I wanted to say return lv_data.
>>How about a LEAVE in the first place?
I'm not in an iterating block : didn't know that you could "leave" a method.
wait - April 1st was yesterday ...
While I understand your objection to extra do blocks, there is also a lot to be said about having a single return statement.
For another data point - I think I've run into this before too. And thought I'd logged a bug for it, but cannot find the bug now.
I have returned stuff in a finally block before too ( ie serialized JSON that will contain different values depending on whether there's an error in the main code or not).
The fact that you get different results when lv_data" is a CHAR variable vs. a LONGCHAR is a bug.
HOWEVER, Mike is correct - you should not use the FINALLY block to do the associated (main) block's work. If for some reason there is an error in the associated block (unexpected in this case) that is thrown out (if you use ROUTINE-LEVEL ON ERROR UNDO, THROW), the RETURN statement in the finally block will conflict with this and the AVM will actually throw the error away!! This is not what you want. It also leaves the door open for code to return one value from the associated block and a different value from the FINALLY block. The FINALLY block value will win, BTW.
If the method were VOID, even a simple RETURN statement in the FINALLY block would have (prior to 11.4) cancelled out any error thrown out of the associated block! We've fixed this case in 11.4.
So it is good practice to make sure that the code in the FINALLY block will never conflict with code in the associated block. It should be used for cleanup, closing queries, freeing memptrs and the like, that should happen whether there is an error or not.
Other languages don't even let you use a RETURN statement in the FINALLY block, which avoids all of these nasty issues. Sigh... Too late now.
Laura
it's never too late, Julian is the only one doing that (Peter just said that so he doesn't feel bad about it)... so, if you can do something to stop him doing it any further please do so :)
yeah, yeah blame me. I've got broad shoulders. I can take it ! Bring it on.
All your bad code are belong to us...
[collapse]On 04/02/2014 05:50 PM, jmls wrote:
>[collapse] From: jmls
> Post: RE: bug with longchar
> Posted in: OpenEdge Development
> Link: http://community.progress.com/technicalusers/f/19/p/9337/35817.aspx#35817
>
> yeah, yeah blame me. I've got broad shoulders. I can take it ! Bring it on.
> All your bad code are belong to us...
I'm you've said that because of the funny include files in that darn
json library you seem to give up on, and I didn't even used that to
please Mike :)
--
m.edu
keep it simple
http://www.ganimede.ro
http://ro.linkedin.com/in/marianedu
medu@ganimede.ro
mobile: +40 740 036 212
skype: marian.edu[/collapse][/collapse]
Looks like you can return the longchar from the finally OR return outside the finally but not both. Good thing you had unit tests!