How to iterate with a cursor and remove elements from container?

Hi,
Can you use a Cursor to remove an element after a test ? For Diary.Appts.Delete (C); below I get:

je.adb:24:30: error: actual for “Position” must be a variable

Which I asume must have to do with tempering checks ? What I want is to remove the duplicated elements when merging two containers.

pragma Extensions_Allowed (all);
with Ada.Streams.Stream_IO, Ada.Containers.Vectors;

procedure Essai is
   package JE is
      type Diary_Type is limited private;
      procedure Delete (Diary : in out Diary_Type;
                        Appt  : Positive);
   private
      package AVP is new Ada.Containers.Vectors (Positive, Integer);
      type Appointment_Array is new AVP.Vector with null record;
      type Diary_Type is
         record
            Appts : Appointment_Array;
         end record;
   end JE;
   package body JE is
      procedure Delete (Diary : in out Diary_Type;
                        Appt  : Positive) is
      begin
         Diary.Appts.Delete (Appt);
      end Delete;

     	procedure Save (Diary : in out Diary_Type;
                      To    : String) is
         use Ada.Streams.Stream_IO;
         File : File_Type;
      begin
         Create (File, Name => To);
         for C in Diary.Appts.Iterate
         when AVP.Element (C) = AVP.Element (C.Next) loop
            Diary.Appts.Delete (C); -- HERE
         end loop;
         Diary_Type'Output (Stream (File), Diary);
         Close (File);
      end Save;
   end JE;
begin
   null;
end Essai;

I think you just need to save the cursor to a variable first then when you exit the loop you can delete it. If you need to delete multiple elements then save the cursors to a vector of cursors and delete them after the finding loop by iterating through the vector and deleting each one

1 Like

I think it would help (us) if you showed us the actual code in which the error occurs. You’ve shown us what looks like essai.adb, but the error is in je.adb.

The procedure you’re calling is

   procedure Delete
     (Container : in out Vector;
      Position  : in out Cursor;
      Count     : Count_Type := 1);
   --  If Position equals No_Element, then Constraint_Error is propagated. If
   --  Position does not designate an element in Container, then Program_Error
   --  is propagated. Otherwise, Delete (Container, To_Index (Position), Count)
   --  is called, and then Position is set to No_Element.

and Position is in out.

Sometimes it helps to iterate in reverse order, but I think you’re going to hit tampering checks.

Since you’re using extensions, you could say

      for C in Diary.Appts.Iterate
        when AVP.Element (C) = AVP.Element (C.Next) loop
         Current : AVP.Cursor := C;
         Diary.Appts.Delete (Current);
      end loop;

(that needs pragma Extensions_Allowed (All)).

Sorry, the code given was enough to replicate the error. The given Delete line causes the given error.
Yes I’ve seen the profile, I just didn’t expect the cursor C to be read-only, like a loop index. So it sufficed to create a copy of the cursor (I can do without this particular ugly extension though…).
Anyway, I got carried away with this cursor thing, I actually didn’t need it at all ! Usually when a solution feels clanky and heavy, there exists a simpler one… A simple index sufficed.

I haven’t tested it out yet, but I was gonna ask you if one can still call the Delete inside the loop? I thought Iterators set the tampering checks on the container forbidding modification (maybe with assertions…not sure)?

EDIT: Or does the new iterator filtering give you some more freedom to do things like this now? I haven’t played with it.

I’d have expected tampering, and that iterator filtering wouldn’t make a difference.

To a Brit, the iterator filtering syntax is ugly and misleading. I looked at OP’s code & thought it must be wrong.

That said, I haven’t tried it, and will wait until I find a need.

1 Like

Ok that makes sense.

I’ve struggled with the syntax as well. I’ve tossed around different formats for it in some examples on godbolt, but I’ve not settled on anything specific yet. The closest I’ve gotten to formatting it in a moderately acceptable way for me is something similar to this:

      for C in Diary.Appts.Iterate
        when AVP.Element (C) = AVP.Element (C.Next) 
      loop
         Current : AVP.Cursor := C;
         Diary.Appts.Delete (Current);
      end loop;

But I haven’t 100% convinced myself yet.