issue with list collection

Posted by GregHiggins on 17-May-2016 14:20

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
            for each btList where btList.sequence >= seq by btList.sequence desc:
                btList.sequence = btList.sequence + 1. 
            ttList.Sequence = seq.
            return true.
        return false.
    end method.

My question: How does ttList get into scope?

All Replies

Posted by Peter Judge on 17-May-2016 14:22

Probable an unintended side effect.
Aka bug.

Posted by GregHiggins on 17-May-2016 14:39

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 */.

Posted by Håvard Danielsen on 17-May-2016 16:11

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.

Posted by GregHiggins on 19-May-2016 22:10

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.

Posted by Håvard Danielsen on 21-May-2016 16:37

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"

This thread is closed