Re: Newbie's question

[Christos Chryssochoidis <C.Chryssochoidis@gmail.com>]

Jeffrey R. Carter wrote:
> 
> Christos Chryssochoidis wrote:
> 
> Apparently you found a compiler error. Some comments on your code:
> 
>>       Result : access Int_Array;
>>    begin
>>       for I in Elements'Range loop
>>          if Predicate(Elements(I)) then
>>             Tmp_List.Append(Elements(I));
>>          end if;
>>       end loop;
>>
>>       Result := new Int_Array(1..Index(Tmp_List.Length));
> 
> This is OK for a little test program like this, but in real code, you 
> have a memory leak from this use of access values and heap allocation 
> that you should want to avoid. You can do that by moving the declaration 
> of Result into Copy_List below:
> 
> Result : Int_Array (1 .. Index (Tmp_List.Length) );

I ruled out the declaration of a local array within the block statement 
Copy_List, because I thought that the compiler would mandate a return 
statement to be at the top level of the function's body, which I 
wouldn't be able to do, since the declared array would be local to the 
block statement. Surprisingly I realized I was wrong; this solution 
compiles fine. The compiler doesn't complain about not finding a return 
statement at the outermost scope of the function.



> 
> You might also find an unbounded array (Ada.Containers.Vectors) more 
> suited than a list here, since you can use the same index for both.

Good point.

> 
>>   Copy_List:
>>       declare
>>          Tmp_List_Cursor : Int_Lists.Cursor := Tmp_List.First;
>>          I : Integer := 1;
>>       begin
>>          while Int_Lists.Has_Element(Tmp_List_Cursor) loop
>>             Result(I) := Int_Lists.Element(Tmp_List_Cursor);
>>             Tmp_List_Cursor := Int_Lists.Next(Tmp_List_Cursor);
>>             I := I + 1;
>>          end loop;
>>       end Copy_List;
> 
> Rather than using a cursor and a loop, I would probably use the Iterate 
> procedure to convert the list into an array.

I tried it. (Code below.) It works fine. Thanks.


Copy_List:
       declare
          Result : Int_Array(0..Index(Tmp_Vector.Length)-1);
          I : Integer := 0;
          procedure Copy(Position: Int_Vectors.Cursor) is
          begin
             Result(I) := Int_Vectors.Element(Position);
             I := I + 1;
          end Copy;

       begin
          Tmp_Vector.Iterate(Copy'Access);

          return Result;
       end Copy_List;





> 
> You could also make Filter recursive and eliminate Result and Tmp_List 
> altogether:
> 
> begin -- Filter
>    if Elements'Length <= 0 then
>       return Elements;
>    elsif Predicate (Elements (Elements'First) ) then
>       return Elements'First &
>              Filter (Elements (Elements'First + 1 .. Elements'Last), 
> Predicate);
>    else
>       return Filter (Elements (Elements'First + 1 .. Elements'Last), 
> Predicate);
>    end if;
> end Filter;

I thought too about doing it recursively, but stubbornly I wanted to 
find a way to do that iteratively, as that's how I would do it in other 
languages I 'm familiar with…

> 
>>    function Greater_Equal_3(Element :Integer) return Boolean is
>>    begin
>>       if Element >= 3 then
>>          return True;
>>       else
>>          return False;
>>       end if;
>>    end Greater_Equal_3;
> 
> This should be
> 
> return Element >= 3;

That's much simpler, I agree.

> 
> Personally, I'd make filter generic rather than passing the Predicate 
> function as a subprogram parameter.
> 

I made another version of my code using a generic subprogram as you 
suggested. That's also an elegant solution.

Thanks very much for all your suggestions.