Add type parameter to JsonArray.stream()

[jo5ef]

Add a type parameter to JsonArray's stream() method to make it easier & more convenient to use JsonArray with the java stream api:

JsonArray arr = new JsonArray()
    .add(new JsonObject().put("price", 123))
    .add(new JsonObject().put("price", 456))
    .add(new JsonObject().put("price", 789));

arr.<JsonObject>stream().mapToInt(i -> i.getInteger("price")).sum());

Also see unit tests for more examples. This does not change or break the semantics of stream() because Stream<Object> will still be returned when no type parameter is specified.

[vietj]

I'm not so much a fan of this as it is a free cast, i.e it prevents the user to check first the type of the object and leads to class cast exception although the compiler said "ok fine" it's safe

[jo5ef]

Hm, but since JsonObject and JsonArray use Map<String, Object> and List<Object> internally, "free cast" is really all around us already anyways. Consider:

arr.add("foo");
arr.getInteger(0); // ClassCastException

Not sure how that is different from

arr.add("foo");
arr.<Integer>stream(); // ClassCastException

Obviously the user expects a specific type when using <T>stream() but the same is true when using the JsonObject.getXXX() methods. This change would just allow to work with Stream<T> instead of Stream<Object> which makes using the java stream api a lot more useful - in my opinion.

[vietj]

right, I do agree on this part, user can do it anyway...

I'm wondering how changing this signature affects compatibility though.

[jo5ef]

I don't think there are any compatibility issues. Because of type erasure in Java, the method signature in bytecode will stay the same and everyone using <Object>stream() can continue to do so without any changes.

[jo5ef]

Any more thoughts on accepting (or rejecting) this PR? I still think this is very useful.

[vietj]

we have plans in Vert.x 4 to have JsonArray extend List<Object> and in this case it would conflict

[jo5ef]

Interesting. You mean JsonArray will implement List<Object>? Or extend ArrayList<Object> or similar? And will JsonObject implement Map<String, Object> then accordingly?

[vietj]

yes this is what we are wanting to do in v4

the idea is that we can use then JsonObject as a Map<String, Object> in Vert.x SQL Client to do row level mapping with DataObject, since there is a way to provide a Function<Map<String, Object> T> in SQL Client, so one could do

client.query(...., SomeDataObjectConvert::fromMap, ar -> { if (ar.succeeded()) { RowSet<SomeDataObject> rows = ar.result(); .... } });

On 17 Oct 2019, at 16:19, Josef Pfleger [email protected] wrote:

Interesting. You mean JsonArray will implement List<Object>? Or extend ArrayList<Object> or similar? And will JsonObject implement Map<String, Object> then accordingly?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eclipse-vertx/vert.x/pull/2120?email_source=notifications&email_token=AABXDCSUVQQTZ54GMESNYXDQPBX5LA5CNFSM4D2LPF5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBQIMKQ#issuecomment-543196714, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCQACHER2RAJF2NUOE3QPBX5LANCNFSM4D2LPF5A.

[jo5ef]

Had a look at the 4.0 features. I don't think there is an overlap with my suggestion. I still find this PR useful.

[jo5ef]

With 4.0.0.CR1 released, I still think this PR is a good idea, very useful, and without any backward compatibility issues. Please let me know if you have any other concerns/questions about this idea, I'd love to see this in Vert.x 4.

[pmlopes]

I understand what the goal is but json arrays are not bound to a single type. So replacing Object with a generic will not work for [1, true, 'a']. It will throw ClassCastException if I'm not mistaken.

[jo5ef]

Absolutely, the same way the other methods currently throw ClassCastExceptions:

arr.add("foo");
arr.getInteger(0); // ClassCastException

(also see discussion above). This PR doesn't change the way stream() (or <Object>stream()) behaves but it makes working with streams a lot more convenient when you know the item type. And I think homogeneous arrays are very - if not more - common.

[vietj]

I think that if you write your own java Function

Function<JsonObject> asJsonObject = val -> (JsonObject)val;

you can do instead

array.stream().map(asJsonObject).mapToInt(...)

I think also you could use Json pointer to achieve the same.

[jo5ef]

Yes, we've had such little helpers in our projects for years now (check the date of this PR), also because we use rxjava heavily. This would allow us to get rid of them.

Just in case there is a misunderstanding: This PR doesn't break anything, the arr.stream() method works exactly as it did before - existing tests did not change. It only adds the possibility to pass an optional type parameter to make it more convenient to use.

[pmlopes]

I see. I had a second look at the PR indeed it doesn't break any API it just defines a generic type in an existing method that was locked to Object. I don't see why not allow it to by type safe.

I let to @vietj to decide :) has he has more insights on the core than me.

[jo5ef]

I'm happy to see that there is a generic asStream method now - thank you! I still think it's a good idea to also make it generic in JsonArray.stream.

[DarwinsBuddy]

Often stumbled upon this issue with java's stream api. would be quiet nice not to cast per element to get a typed element :)