I'm looking at the following code from List.cls in the OpenEdgeCore library:
method public logical Add(seq as integer, obj as Object ):
define buffer btList for ttList.
if super:Add(obj) then
do:
for each btList where btList.sequence >= seq by btList.sequence desc:
btList.sequence = btList.sequence + 1.
end.
ttList.Sequence = seq.
return true.
end.
return false.
end method.
My question: How does ttList get into scope?so maybe something like
for each btList where btList.sequence eq 0: assign btList.sequence = seq when tList.objectref:Equals(obj) . if btList.sequence eq seq then leave. end /* for each */.
I suspect the super:Add put the ttList in scope when this was implemented, but this is apparently no longer the case.
The fix seems good.. It should in theory work even without the when, but it is better to be on the safe side and keep it.
This is the method called by super:Add(obj):
method public logical Add( newObject as Object): if valid-object(newObject) then do: mhDefaultBuffer:buffer-create(). assign mhField:buffer-value = newObject Size = Size + 1. mhDefaultBuffer:buffer-release(). return true. end. return false. end method.
As we can see, mhDefaultBuffer:buffer-release() is what makes ttList unavailable.
Obviously, this line of code was added after the List class was developed and (presumably)
tested. My favorite development tools company has this feature called ABLUnit which
can be used for unit regression testing; it might be the kind of thing you folks would want to
look into.
My personal preference is for the buffer-release to stay; I am not a fan of leaving scoped
records lying about.; though I have created persistent procedures and some classes whose
sole purpose was to manage a temp-table where I sometimes allowed global scope.
Regardless of what you choose, I think unit tests for every class in the library is required.
Yes, this is very likely what happened. As far as I can see the bug also exists in AddAll(seq as int,c as ICollection) and AddArray(seq as int, obj as Object extent) . Add(obj as Object ) avoids the problem by calling findfBufferUseObject. This is of course also a method with global scope, but in this case this is the sole purpose of the method and the name reflect the behavior. It is also a protected method.
How to scope temp-tables in procedures or classes is both a difficult and interesting subject. As you point out there are certain types of objects where a global scope makes sense. This is probably not one of those cases. There are few or no benefits from keeping the temp-table available between the public operations.
But I also think that sub-classes should be able to share scope with the super class. Having to execute a find after the call to super:add is a bit ugly and probably also expensive (in percentage). Maybe some dedicated internal addBufferUseObject method is the solution. (This should probably exist also in the sub-classes)
We need to look into the ABLUnit product that you mention. A test suite for ABL objects would be extremely useful for these classes that have so many use cases. The fact that they are used at run-time means they need to be tested as thoroughly as the language itself.
edit : corrected to "where a global scope makes sense"