RFC: Primitive List API

[cmelchior]

With work in Core slowly picking up speed regarding Primitive List support, we need to start thinking about how the public API is going to look like. Also, so it can guide the underlying implementation.

This document tries to flesh out the various proposals that have been discusse so we can get feedback on the direction as well as guide Cores design decisions.

Thoughts @realm/java ?

References #575

[zaki50]

Shouldn't we have public class PrimitiveLongRealmQuery extends PrimitiveRealmQuery<Long>?

[cmelchior]

Ups, yes.

[kneth]

I prefer option 1. From a user's perspective is it easier: RealmList<Long> and RealmList<Cat> are "just" lists. Forcing the users to think about details of the types (Long and Cat) feels wrong.

We are pushing the fluent interface of the query engine. RealmQuery is becoming more and more bloated, and I think it is worth considering switching to the same query system as Realm JS.

[Zhuinden]

I think laxing the extends RealmModel and just using Object instead will help with and is actually a requirement of #1694

But if I have to write Realm queries by hand in Strings, I may as well go back to SQLite

EDIT from the future: although I wouldn't mind an optional raw query api.

[cmelchior]

Depending on how Type Adapters are implemented that might not be necessary, but it would probably make a lot of things easier. Good point.

Regarding the query system. We won't remove the typed API, but it is starting to squeak a bit under the weight of operators. So a string based API would live next to the typed one. At least that is the current idea.

[nirinchev]

For .NET, option 1 will probably work better - we use the standard IList<T> on which we can relax the generic constraint to allow all types, naturally, issuing a compile time error if used with anything other than RealmObject or valid primitive type. Since we're using LINQ, the RealmQuery method explosion doesn't concern us. The API will be type-safe, but it might be harder to expose some functionality, e.g. like searches.

From an end-user POV, I too like 1 more, due to @kneth's argument.

cc @kristiandupont @fealebenpae

[nhachicha]

I'm in favour of relaxing the type requirement on RealmList. As discussed with Zaki we can still check at compile time (using the annotation processor) that the user doesn't abuse the generic, by using an unsupported type.

Example: we can detect at compile time this RealmnList<BigInteger> and throw an exception

[cmelchior]

Good point. We should be able to catch all wrong types used in the annotation processor. Didn't think of that 👍

If we relax the type parameters, I'm also secretly playing around with the idea of splitting out DynamicRealmObject from RealmObject since having to deal with both of them everywhere is a big hassle and put some rather annoying restrictions on what we can do in both classes.

[Zhuinden]

Ah, then ability to get Realm / DynamicRealm from their corresponding classes will be a possibility :3

[zaki50]

OK, I will start adding APIs with option 1.

[zaki50]

In regards to Design1, 4a, how about accepting null as field name for primitive queries instead of adding new methods? @realm/java

[beeender]

In regards to Design1, 4a, how about accepting null as field name for primitive queries instead of adding new methods?

I can accept it. Feels kind of nature to me.

[cmelchior]

I think the 4a) option was not thought through. Querying would have to work like you would do with a RealmIntegerWrapper today but without the linked query. Consider

public class ListDemo extends RealmObject {
  public RealmList<String> firstNames;
  public RealmList<String> lastNames;
  public String name;
}

// Currently with a wrapper type it would be 
realm.where(ListDemo.class).equalTo("firstNames.value", "John").findAll(); 

// With primitive support it would be
realm.where(ListDemo.class).equalTo("firstNames", "John").findAll();

// Breaks if you have multiple primitive lists
realm.where(ListDemo.class).equalTo("John").findAll();

[cmelchior]

Ah crap, now I remember:

demo.firstNames.where().equalTo(null, "John");

So I guess you are advocating for 4B with null as the secret keyword? What are you thoughs on the tradeoffs discussed?

One thing missing from the design doc description is that the extra methods are only needed when querying the list directly. Something which I assume is done less frequently than the model in general. This at least means that adding a lot of extra methods have a bigger drawback

[bmeike]

I do not love the type relaxation. I freely admit that I haven't had time to consider the consequences of the alternatives, but I really don't love Variant #1.

[bmeike]

Right. I can see that I am in a minority, here, but this is what types are for. I think this argument, all by itself, should be a stopper.

[cmelchior]

Note, that even with types, the current system <? extends RealmModel> isn't perfect as a Schema can restrict valid types even further.

[bmeike]

Noted.

[bmeike]

Note that, to the contrary, type safety is still guaranteed for all of RealmList. It just isn't guaranteed here.

[zaki50]

I made two PRs to define Primitive List API.
One is Design 1 (#5145) and another is Design 2 (#5139).

I was thinking that I had a problem in Design 1, but now I solved most of them and now I'm leaning towards Design 1.

[cmelchior]

I strongly prefer Design 1 as well. There are a few caveats like findFirstAsync() not working on value lists and nullability annotations forcing null checks more places than strictly needed, but I think that is acceptable.

[zaki50]

OK, I start implementing the feature on Design 1.
I split the PR into smaller PRs.

#5145: Part1: API changes (This PR)
#5150: Nullability changes
#5151: Solve compilation errors
#5152: Tests
#5168: RealmList's type check in Annotation Processor

And I'll add a PR for annotation processor.

[Zhuinden]

$ isn't that good because you need to put it in backticks in kotlin.

EDIT: wait, this is in a String. What was I even thinking?

[Zhuinden]

oh, so that's what "nested subtables" means.