Re: JSONArray "Object remove(int index)" behavior not consistent with other methods

Hello, Ben!

Great catch!

I suggest we make the change to throw an exception in Tapestry 5.9.0
with a big warning in the release notes.

Cheers!

On Sat, Jul 29, 2023 at 7:55 AM Ben Weidig <[email protected]> wrote:
>
> Hi there!
>
> I was trying to add "static empty()" methods to JSONArray and JSONObject to
> reduce the creation of new instances if we know it's supposed to be empty,
> like Collections does with emptyList()/emptyMap()
>
> Adding the methods isn't the issue (creating private constructor accepting
> the underlying storage, using the appropriate Collections.empty... method),
> but what I discovered while writing the unit tests.
>
> The JSONArray method "Object remove(int index)" doesn't explode when used
> with an invalid index.
> The documentation states this behavior, but I think that's wrong or at
> least inconsistent with the rest of the type.
> There's even a JSONArrayIndexOutOfBoundsException that's used by "Object
> get(int index)" and, therefore, by all other index-based methods.
>
> This discrepancy in expected behavior seems weird to me, and if we decide
> to elevate JSONArray to implement List<Object> in the future, as it's
> almost there anyway, it goes against the documented contract of List<T>.
>
> However, it would be a possible breaking change for people relying on the
> non-explody behavior.
>
> Thoughts?
>
>
> Cheers
> Ben



-- 
Thiago