Delphi: Goto not considered harmful in this case?

4
votes

Alright, so you have a TObjectList instance. You want to loop through the items in it and delete some of the objects from the list. You can't do this:

for I := 0 to ObjectList.Count - 1 do
  if TMyClass(ObjectList[I]).ShouldRemove then
    ObjectList.Delete(I);

...because once you delete the first object the index counter I will be all wrong and the loop won't work any more.

So here is my solution:

Again:
  for I := 0 to ObjectList.Count - 1 do
    if TMyClass(ObjectList[I]).ShouldRemove then
    begin
      ObjectList.Delete(I);
      goto Again;
    end;

This is the best solution I've found to this so far. If anyone has a neater solution I'd love to see it.

3
delphigoto
Wait... Does that mean that in Delphi, ObjectList[k] has complexity O(k) (when ObjectList is a list)? In this case, your algorithm has the wrong complexity to start with. If you are handling lists, you should have the proper operations on lists and not be writing for loops, and then you would not be wondering about goto (although you may be wondering about exceptions). – Pascal Cuoq
I'm looping because I want to remove several items from the list. Otherwise I'd just use ObjectList.Remove(ObjectToRemove); – David
@David I understand that much, and perhaps I just shouldn't be mucking with questions about languages I am not that familiar with, but if ObjectList[k] has complexity O(k) (and I suspect it has), your implementation language is forcing you to write algorithms with the wrong complexity. It's time to switch... There are languages in which you can remove elements you ShouldRemove from a list in O(length). – Pascal Cuoq
ObjectList[k] accesses the object directly. It doesn't need to loop through 0..k to find the correct object. – David
@david Fair enough, I'm out of my depth here (although I would like to see the implementation of that data structure that lets you remove some elements but has O(1) access time to the nth element). – Pascal Cuoq

3 Answers

30
votes

Try this instead:

 for I := ObjectList.Count - 1 downto 0 do
   if TMyClass(ObjectList[I]).ShouldRemove then
     ObjectList.Delete(I);

That looks like a particularly bad use of goto, jumping out of the for loop like that. I assume it works (since you're using it), but it would give me the willies.

7
votes

You can also use

  I := 0;
  while I < ObjectList.Count do begin
    if TMyClass(ObjectList[I]).ShouldRemove then ObjectList.Delete(I)
    else Inc(I);
  end;
0
votes

The only valid use of Goto I've seen is the one supplied in Delphi's help.

for I := 0 to something do
begin
  for J := 0 to something do
  begin
    For K := 0 to something do
    begin
      if SomeCondition then
        Goto NestedBreak
    end;
  end;
end;
NestedBreak:

Though Goto could be avoided in that exemple by moving the loop in a local function and using EXIT, for exemple. If a subfunction is not acceptable, you can still do that:

for I := 0 to something do
begin
  for J := 0 to something do
  begin
    For K := 0 to something do
    begin
      if SomeCondition then
      begin
        GottaBreak := True
        Break;
      end; 
    end;
    if GottaBreak then Break;
  end;
  if GottaBreak then Break;
end;

This is just sligthly less optimal.

I have yet to see a single other situation where a Goto would be the best solution.(Or any good at all).

Goto in itself is NOT bad. It's a flow control command just like EXIT, BREAK or CONTINUE. Except that those other are restricted to specific situations and are managed by the compiler correctly. (With that being said, some programmer I spoke with consider those as being as harmful as Goto, a view I don't share) Goto being unrestricted, the things you can do with it can have very negative impacts. Anyway, I think I went a bit beyond the scope of the question already. ^_^