Skip to content

Commit a9299a2

Browse files
authored
[Mono.Android] JavaList.Add should allow duplicates. (#9751)
Fixes: #9675 Context: https://github.com/xamarin/monodroid/commit/0e782afc16fe4b7197c800864d6c10520905fe28 Context: https://github.com/xamarin/monodroid/commit/8d38265ba00678ff3f3dd6edd8ceb10a97b55abd Context: https://github.com/xamarin/monodroid/commit/be4087f038957a36a0bb3b9f9dc401c65ef379ca Binding is a historically hairy business, and sometimes we live with the results of those choices decades later. Case in point: what should be done about `java.util.List<E>`? Some time around 2010-Sep-7 in xamarin/monodroid@0e782afc was a "Generic and Collections restructuring", which resulted in the *removal* of `java.util.List` and other interfaces from `Mono.Android.dll`. The idea was that bindings would instead use `Android.Runtime.JavaList` and other types instead. This was the continued state of affairs when [Bugzilla 8112][0] was filed, which was an issue binding DroidText, which was simplified into [Bugzilla 8134][1]. The underlying problem was that some type in DroidText implemented `java.util.List`, which wasn't bound and instead was replaced with `JavaList`, but the generated bindings would attempt to override and implement `java.util.List` methods such as `Add()` on `JavaList`, which didn't exist, resulting in compilation errors. The fix in 2012-Nov-2 in xamarin/monodroid@8d38265b was to add all `java.util.List` methods onto `JavaList`. Fair enough, but this was also a time with minimal code review, and the fix has a couple of obvious problems in retrospect: partial class JavaList { public virtual bool Add (int index, Java.Lang.Object item) { if (Contains (item)) return false; Add ((object) item); return true; } public virtual bool Add (Java.Lang.Object item) { return Add (0, item); } } Firstly, why is `Add(int, Object)` calling `Contains(item)`? This meant that duplicate values couldn't be added, even though that is something that Java would allow. Secondly, why is `Add(int index, Object item)` *ignoring* the `index` parameter? Thirdly, why is `Add(Object)` always inserting at index 0? That doesn't really make sense, and the only reason it hasn't resulted in `JavaList.Add()` invocations resulting in *reversing* the order of `Add()` calls is because the `0` was ignored (2). Fix all three of these issues. The underlying problem of "`java.util.List` isn't bound!" was later resolved in 2013-Mar-11 by xamarin/monodroid@be4087f0, which stopped removing `java.util.List` from `Mono.Android.dll`. [0]: https://web.archive.org/web/20210922182648/https://bugzilla.xamarin.com/81/8112/bug.html [1]: https://web.archive.org/web/20210925214551/https://bugzilla.xamarin.com/81/8134/bug.html
1 parent a941e34 commit a9299a2

File tree

2 files changed

+35
-4
lines changed

2 files changed

+35
-4
lines changed

src/Mono.Android/Android.Runtime/JavaList.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -546,14 +546,13 @@ public static IntPtr ToLocalJniHandle (IList? items)
546546
//
547547
public virtual bool Add (Java.Lang.Object? item)
548548
{
549-
return Add (0, item);
549+
Add ((object?) item);
550+
return true;
550551
}
551552

552553
public virtual bool Add (int index, Java.Lang.Object? item)
553554
{
554-
if (Contains (item))
555-
return false;
556-
Add ((object?) item);
555+
Insert (index, (object?) item);
557556
return true;
558557
}
559558

tests/Mono.Android-Tests/Java.Interop/JavaListTest.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,38 @@ namespace Java.InteropTests
1515
[SetUp]
1616
public void Setup () => list = new T ();
1717

18+
[Test]
19+
public void Add ()
20+
{
21+
list.Add ("foo");
22+
Assert.AreEqual ("foo", list [0]);
23+
24+
// Ensure duplicates are allowed.
25+
list.Add ("foo");
26+
Assert.AreEqual (2, list.Count);
27+
Assert.AreEqual ("foo", list [1]);
28+
}
29+
30+
[Test]
31+
public void AddWithIndex ()
32+
{
33+
list.Add ("Apple");
34+
list.Add ("Banana");
35+
list.Add ("Cherry");
36+
37+
// Ensure index is respected.
38+
list.Add (3, "Grape");
39+
list.Add (2, "Blueberry");
40+
list.Add (4, "Fig");
41+
42+
Assert.AreEqual ("Apple", list [0]);
43+
Assert.AreEqual ("Banana", list [1]);
44+
Assert.AreEqual ("Blueberry", list [2]);
45+
Assert.AreEqual ("Cherry", list [3]);
46+
Assert.AreEqual ("Fig", list [4]);
47+
Assert.AreEqual ("Grape", list [5]);
48+
}
49+
1850
[Test]
1951
public void Count ()
2052
{

0 commit comments

Comments
 (0)