Enumerators and overly-defensive programming

Jeroen Pluimers has been blogging about implementing enumerators, which are those things that allow your code to hook into the for-in loop syntax introduced in D2005. His use of the concept so far seems fairly straightforward, but partly because of that, I’m inclined to take issue with this sort of thing:

function TComponentEnumerator.MoveNext: Boolean;
begin
 Assert(FMaxValue = FComponent.ComponentCount - 1, 'ComponentCount changed during enumeration');
 // ...
end;

OK, I realise the VCL’s default enumerators haven’t been written to expect the enumerated items may change during the enumeration, but why not?  Cf. the following, from my Exif library code:

function TExifSection.Enumerator.MoveNext: Boolean;
begin
if (FCurrent <> nil) and (FIndex < FTags.Count) and (FCurrent = FTags.List[FIndex]) then Inc(FIndex); Result := FIndex < FTags.Count; if Result then FCurrent := FTags.List[FIndex]; end; [/sourcecode] The enumerator then has the following constructor (its Current property directly reads the FCurrent field): [sourcecode language='delphi'] constructor TExifSection.Enumerator.Create(ATagList: TList); begin FCurrent := nil; FIndex := 0; FTags := ATagList; end; [/sourcecode] Implementing the enumerator like that allows it to be used like this: [sourcecode language='delphi'] for Tag in ExifData[esDetails] do if Tag.DataType = tdUndefined then Tag.Delete; [/sourcecode] Given it wasn't difficult to add this extra flexibility, where's the benefit in actively going against it...?

4 thoughts on “Enumerators and overly-defensive programming

  1. Basically , it is a choice based on the clearness of the semantics.

    What would be the good semantics when deleting members at position 2, when you are at position 4? Do you continue with position 6? Do you add logic to go to position 5?

    Since the semantics are unclear, I guarded against deletion or insertion (though not on modification, that would require maintaining a version stamp).
    In fact is it similar to the .NET implementation of “InvalidOperationException: Collection was modified; enumeration operation may not execute” . at System.ThrowHelper”.

    Another solution would have been be to document the semantics upon deletion, insertion or modification of the underlying list.

    Choices, choices 🙂

    –jeroen

    • Yes, I agree the way enumerators are implemented allows for a lot of choice on the implementor’s side. Even so, I wouldn’t say the flaw in my preferred way is exactly what you suggest, since if a member at position 2 is deleted when you’re currently at position 4, then the third check in my code — namely, of (FCurrent = FTags.List[FIndex]) — would fail and FIndex would not be incremented, correctly causing the next element enumerated to be what was originally at position 5 and is now at position 4. Where unstructured behaviour would arise, however, is when more than one element earlier on in the sequence is deleted.

      That said, on reflection, I’d probably prefer any checking code do be in TElement/TContainer.Delete (or equivalent) rather than TEnumertor.MoveNext – better get an exception raised when you actually try to delete an element you shouldn’t be deleting rather than only afterwards when you go to retreive the next one.

  2. I have thought a while about this, and think it comes down to this:

    If you have influence on the existing classes, then it makes sense to make the modification methods aware if there are enumerations running, and then fail.

    If you do not have influence, then you should add detection code to the enumeration. I know that the .NET framework does that, and wonder how the Java SDK is doing it.

    –jeroen

    • ‘If you do not have influence, then you should add detection code to the enumeration.’

      I don’t know — the more the detection code is watertight, the more checks will be made at every MoveNext whether something has been done to the enumerated list or not. Purely in terms of efficiency there’s something to be said for the default VCL enumerator classes’ simple approach I think.

Leave a comment