Use collection interfaces instead of concrete types in generated Java classes

[roded]

We're using 0.11-SNAPSHOT with serialization functionality.

I have the following field in a ksy:

  - id: ecb_addresses
    doc: User ECB address list
    type: u4
    repeat: expr
    repeat-expr: ecb_count

Which generates the following field in its associated Java class:

private ArrayList<Long> ecbAddresses;

Is there a reason for using the concrete java.util.ArrayList in the generated class instead of the java.util.List interface (short of the _read() method's initialization of said field)?

When setting this field, I'd like to be able to pass it any java.util.List and not just an explicit java.util.ArrayList.

[generalmimon]

@roded:

Is there a reason for using the concrete java.util.ArrayList in the generated class instead of the java.util.List interface (short of the _read() method's initialization of said field)?

No particular reason, except that java.util.ArrayList has seemingly always been used. Strictly speaking, changing it to java.util.List now is a breaking change, though it presumably wouldn't affect many people.

When setting this field, I'd like to be able to pass it any java.util.List and not just an explicit java.util.ArrayList.

Just out of curiosity, which other implementation of java.util.List would you like to use?

[roded]

Thanks for taking a look at this @generalmimon .

Just out of curiosity, which other implementation of java.util.List would you like to use?

It's not so much that I'd like to use a different implementation, but I'd like to use values returned from interfaces which return a java.util.List.

For example, when calling <stream>.collect(Collectors.toList()); I don't really care about the java.util.List implementation and I'd like to avoid the extra step in copying it into an java.util.ArrayList. Another example is the Guava library's ImmutableList or java.util.Collections.unmodifiableList()'s UnmodifiableList - I don't really want to create a multable list for serializing a Kaitai struct if I can help it.

[GreyCat]

I agree that it generally makes sense, especially in context of serialization (=supplying structures) as opposed to deserialization (=consuming whatever you've been given).

With the advent of serialization into the master branch, maybe it's a good idea to correlate that with these changes (at least we're not breaking things twice).

It will be likely still quite non-trivial, as you'll end up having to solve problems with construction of all these arrays, e.g. instead of

        this.aint = new ArrayList<Long>();
        for (int i = 0; i < 4; i++) {
            this.aint.add(this._io.readU4le());
        }

it will be more complicated something like:

    List<Long> aint;

// ...

        ArrayList<Long> tmpAint = new ArrayList<Long>();
        this.aint = tmpAint;
        for (int i = 0; i < 4; i++) {
            tmpAint.add(this._io.readU4le());
        }

[generalmimon]

@GreyCat:

it will be more complicated something like:

    List<Long> aint;

// ...

        ArrayList<Long> tmpAint = new ArrayList<Long>();
        this.aint = tmpAint;
        for (int i = 0; i < 4; i++) {
            tmpAint.add(this._io.readU4le());
        }

I don't think so, List definitely has all the methods we need, including add. ArrayList has very few additional methods on top of List (it seems that void ensureCapacity(int) and void trimToSize() are the only extra public methods that List doesn't have).

You might think "what about immutable collections - surely List wouldn't have mutating methods because of them", but Java solves this by documenting the mutating methods as "(optional operation)" and having them throw UnsupportedOperationException in case the particular collection doesn't support them. For instance, see add:

boolean add(E e)

Appends the specified element to the end of this list (optional operation).

(...)

Throws:
UnsupportedOperationException - if the add operation is not supported by this list

---

So I believe the change from ArrayList to List will be seamless for both us and the users, actually.

I said that it might be a "breaking change", which is technically true (for example, if there is some user application code out there that takes a list from an instance of a KS-generated class and saves it to an ArrayList variable, then this code will no longer work), but I believe it will affect a minimal number of users and it should be extremely straightforward to "migrate" (just use List instead of ArrayList everywhere in the application code, which is a good practice anyway).

Calls to ensureCapacity or trimToSize would also no longer work, but it seems very unlikely that anyone would need them.

[GreyCat]

@generalmimon Makes sense, thanks for thorough investigation! Then it will be likely simpler.