Some code errors in Island and RTL

I have found a three code errors and a for each bug when poking around in Oxygene with Windows as Target.

I checked the source in IslandRTL and RTL2.

SingleLinkedList does not update count when adding items.

In IslandRTL in Monitor.pas we have

SingleLinkedList = class
where
T is class;
private
fTail, fHead: SingleLinkedListNode;
fCount: Integer;
public
method AddLast(val: T);
method RemoveFirst: T;
property Count: Integer;
end;

property Count does not reference fCount.

I added property Count: Integer read fCount; and it fixed the count issue.

In RTL2 LinkedList AddLast method didn’t actually add any nodes.

In LinkedList.pas

method AddLast(aNewNode: LinkedListNode);
begin
aNewNode.Previous := fLast;
aNewNode.Next := nil;

fLast := aNewNode;

if not assigned(fFirst) then
  fFirst := aNewNode;
inc(fCount);

end;
needed 

if assigned(fLast) then
fLast.Next := aNewNode; between aNewNode.Next := nil and flast := aNewNode.

between line 151 and 153 in the source.

And there were some padding issues in Convert.pas in method Convert.ToString(aValue: Double; aDigitsAfterDecimalPoint: Integer := -1;
aMinWidth: Integer := 0; aLocale: Locale := nil): not nullable String;

Lines 253 - 257 were/are

if aMinWidth > 0 then
result := result.PadStart(aMinWidth, ’ ')
else if aMinWidth > 0 then
result := result.PadEnd(aMinWidth, ’ ');
end;

the condition test is the same as is the assignment of aMinWidth instead of -aMinWidth.

Should be

if aMinWidth > 0 then
result := result.PadStart(aMinWidth, ’ ')

else if aMinWidth < 0 then
result := result.PadEnd(-aMinWidth, ’ ');
end;

I made these changes and compiled and added that reference to a project to test and they seem to work ok.

Finally the for each loop structure does odd things in static arrays.

array[3..6] of Integer := [10, 20, 30, 40]; using a for each loop on this will give 6 results and the erroneous two in my test were either 0’s or a repeat of the first two array entries.

another : array[0..3] of Integer := [10, 20, 30, 40]; gives an erroneous result leaving off the last item.

array[1..4] of Integer := [10, 20, 30, 40]; In these simple tests returned the correct number of items.

I hope this is clear enough and useful. Although I doubt that there is a long line of coders waiting for SingleLinkedList to start working properly.

Oops, thanx. fixed.

yup, it failed to set “oldLast.Next”; i fixed it as such:

    method AddLast(aNewNode: LinkedListNode<T>);
    begin
      aNewNode.Previous := fLast;
      fLast:Next := aNewNode;
      aNewNode.Next := nil;
      fLast := aNewNode;
      if not assigned(fFirst) then
        fFirst := aNewNode;
      inc(fCount);
    end;

Honestly not a use pattern I would have expected (negative to pad at the end), but it looks like whoever did write this did intend that, and just messed it up. Fixed.

Can i see a more concrete test case for this? works fine for me here:

Thanks, at first I thought the formatting was a fix but couldn’t see how. I think your test case is a .Net project and I have been working in Native Windows which I didn’t make clear enough. The for each loop does work in .Net, but not Island or Island using RTL. This is Island

And this is .Net

Thamx, for the extra info; i’ll test this against native Windows.

Logged as bugs://E27403.

reproduced; this one is a compiler bug, it seems.

Yep. Thanks.

1 Like