realm / realm-js

Realm is a mobile database: an alternative to SQLite & key-value stores
https://realm.io
Apache License 2.0
5.62k stars 558 forks source link

Add support for counters #4089

Open kneth opened 2 years ago

kneth commented 2 years ago

Problem

Realm Core supports a special case of integers: counters. Counters are in particular useful for synced Realms.

See also https://github.com/realm/realm-core/issues/5056

Solution

We'll be adding a new property type named "counter", which will use the "int" core type underneath but return instances of a Counter class instead of raw number.

declare class Counter {
  public get value(): number;
  public increment(by = 1);
  public decrement(by = 1);
  public set(value: number);

  // Implements https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/valueOf
  // this would allow using an instance of `Counter` to be coerced into a `number`.
  public valueOf(): number {
    return this.value;
  }
}

Because we cannot overload the increment operators ++, +=, -- and -= explicitly in JS (these simply call "set" / assignment underneath), we suggest throwing if these are used, to prevent users from relying on these operators which wouldn't use the proper counter semantics.

We discussed implementing a heuristic (if the setter is invoked just after the getter, perhaps users are performing a +=), but we wouldn't be able to distinguish that from a *= 2, in which case it would be misleading to use the counter semantics.

How important is this improvement for you?

No response

kraenhansen commented 1 month ago

I wonder what an intuitive API would look like.

We cannot overload the ++ or += operators (because that's a limit in JS), and as I see it we have (at least) two options:

  1. Calculate the difference to the existing value when performing an assignment (which is the underlying primitive called when users perform ++ or += on a Number) and use this to call into core to perform the increment. We should consider exposing this behind a flag on the "property schema" passed when opening the Realm, to allow users to opt into this behavior, per property.
  2. New increment and decrement instance methods on Realm.Object, taking the property name and an optional value to increment by (defaulting to 1):
    car.increment("passengers", 2); // Two people are stepping in
    car.decrement("passengers"); // One person is stepping out

    One drawback of this approach is that it interfeer with the property namespace owned by the user through the schema (i.e. users won't be able to use the feature and have properties named "increment" or "decrement" at the same time).

takameyer commented 1 month ago
  1. I think this is the most intuitive, but definitely needs to be something the user opts into. The question on this is how to opt-in? One way would be to define a new data type and have that wrap "int". Or we would add an option to the property object.

  2. I assume this would be easier to implement, but not as pretty as option 1. Alternatively, the increment/decrement methods could be a static function in the Realm namespace (or Realm.Object) that one could call, to avoid adding functions to the users defined object.

kraenhansen commented 1 month ago

One way would be to define a new data type and have that wrap "int". Or we would add an option to the property object.

I agree - it seems we have (at least) two alternative ways of declaring the "counter" behavior on an int:

1.a type: "counter"

A new counter type that would use int on the core level but use the Obj#add_int API when performing assignments.

// shorthand form
schema: [{
  name: "Car",
  properties: {
    passengers: "counter"
  }
}],
// long form
schema: [{
  name: "Car",
  properties: {
    passengers: {
      type: "counter",
      indexed: true,
    }
  }
}]

Pros:

Cons:

1.b) counter: true

This which can be set on the property schema of a type: "int" property:

schema: [{
  name: "Car",
  properties: {
    passengers: {
      type: "int",
      indexed: true,
      counter: true, // would throw if used on types other than `int`
    }
  }
}]

Pros:

Cons:


I'm probably leaning slightly towards 1.a.

takameyer commented 1 month ago

I think 1.a would be best as well. The other SDKs have different ways of codifying the counter. I could actually only find docs in Kotlin (which implements a separate type) and .NET.

The user will probably not be confused, since they won't be using multiple SDKs. IMO best to just focus on what is most intuitive for the platform.

takameyer commented 1 month ago

We will also need to add some good documentation around this. It should be clear that simply assigning a number to a counter will cause all previous increments and decrements. Assignment essentially resets the counter to a value. So the API for a counter would be:

kraenhansen commented 1 month ago

As I hinted in my first comment, we don't have ways to explicitly intercept increments and decrements (++, --, += and -=) except through a setter, which would also be hit when doing regular assignments.

We might be able to apply a heuristic which would hook into the getter and detect an increment / decrement if the getter is accessed just before the setter is called, in the same synchronous execution block 🤔 I'm uncertain if this would have any drawbacks or be too weird of an API, but I can't really think of any other ways to achieve increments, decrements and explicit assignment, through operators.

nirinchev commented 1 month ago

Can we have this return a custom type, similarly to what we do in C#? If we could, then we'd be able to define functions users can use to increment/decrement rather than the ++/-- operators.

I'm not sure if that'd work in js, but roughly something like:

class Counter {
  private objKey: ObjKey;
  private colKey: ColKey;

  public get value(): number {
    return core.getValue(this.objKey, this.colKey)
  }

  public increment(value: number) {
    core.increment(this.objKey, this.colKey, value)
  }
}

Counter.prototype.valueOf = function() {
    return this.value;
}

That way, you could still use that if you need to do print(obj.myCounter + 5), but you could also do obj.myCounter.increment(5).

kraenhansen commented 1 month ago

@nirinchev that's a cool approach and a great use of the valueOf method - the only drawback I can think of is that typeof obj.myCounter would be "object" and not "number". I like how it's more explicit (not relying on the heuristic I mentioned earlier) and that the increment method doesn't bleed into the users namespace of property names 👍

kneth commented 1 month ago

With the class Counter approach, I imagine we need to annotate it in the schema. The approach cars.increment("passengers", 2) will not (any int property is allowed), and it will make it more flexible. Even if passengers is mixed, we can validate that it is an int before calling Obj::add_int().

takameyer commented 1 month ago

I also just though to of a Con for the counter type. If we are expecting a counter to be part of a mixed data type, it may difficult to determine what action to take when using ++ or +=.

nirinchev commented 1 month ago

The ++ and += operators are inherently not something we want/can support due to the way they work - they roughly expand to foo = foo + 1, which is assignment rather than an increment operation. They also are not supposed to modify the underlying value, whereas with counters we want exactly that.

kraenhansen commented 1 month ago

not something we want/can support

We could probably support it, via the heuristic I outlined above - if we want to is a different question. I do see some value in that: I'd expect some users to gravitate towards that way of incrementing a counter and be surprised that it works (it goes up), but doesn't uphold the same guarantees as calling an increment() method.

They also are not supposed to modify the underlying value

Why is that? Calling car.passengers++ or car.passengers += 1 should update the value in the database?

nirinchev commented 1 month ago

They also are not supposed to modify the underlying value

Typically, it's because the value that the ++ operator operates on is immutable (i.e. an integer). The reason why car.passengers++ modifies the car is because this expands to car.passengers = car.passengers + 1 - i.e. it's assigning the result of the calculation to the original variable/property. I don't believe it's possible in JS, but in languages where ++ is overloadable, it'd be bad practice to implement it in a way where it mutates the object it operates on, rather than returning a new value, which is then assigned to the variable that used to hold the object. If we had a Counter class and could overload ++, it'd be a bad practice to mutate the database at that point, but we should rather return some value, that when assigned to the original variable/property tells it to increment itself by some value. That's a bit theoretical though and not something I want to get too bogged down in.

I feel the more important discussion is whether silently replacing the behavior of ++ with calling .increment() is the right thing to do, because of the assignment semantics that the operator has. We'd be walking a fine line, trying to predict user intent with heuristics, which may backfire in obscure ways. It's also possible I don't have enough js knowledge to appreciate how smart the heuristics can be, but I can easily imagine a situation where a user does something like company.projectedRevenue = company.projectedRevenue * 1.2, which we could interpret as an increment rather than an assignment. There may be solutions out there, I'm just a bit conservative when it comes to adding magic to the SDK and would typically err on the side of doing something dumb and obvious rather than something very clever that may catch users by surprise.

kraenhansen commented 1 month ago

I don't believe it's possible in JS, but in languages where ++ is overloadable, it'd be bad practice to implement it in a way where it mutates the object it operates on

Operatores aren't overloadable, as such, but implementing proxy handlers for get + set could get us a long way towards something that behaves like it.

I think I understand you now, by "underlying value", you ment the primitive, I thought you ment the car.passengers (in my example). I agree that's not desirable for the primitive itself to update (nor is it possible in JS), as they are always copied and never accessed by reference.

a user does something like company.projectedRevenue = company.projectedRevenue * 1.2, which we could interpret as an increment rather than an assignment.

I agree - that would indeed be unfortunate 🤔

kraenhansen commented 1 month ago

I've updated the description above with my takeaway of the discussions above ☝️

elle-j commented 3 weeks ago
  1. My 2¢, from the perspective of aiming to provide an unambiguous interface where the user's assumptions match the semantics, I could see it as somewhat problematic to try to intercept get/set calls from using the operators in order to make it behave like a counter. E.g. I see it as reasonable for a user to assume that it'd act exactly like a JS Number, but also reasonable for another user to assume that it'd act like a counter, and for a third to not know what to assume. Essentially, I think allowing that would cause some discrepancy between assumption and semantics, while having distinct APIs like increment(), etc. removes that.

  2. Regarding type: "counter" vs counter: true.

    • If we ultimately decide to only support modification via specific APIs like in the current issue description, I think having type: "counter" would better distinguish this from a regular int, as the way a user would use the type would be quite different for some expressions.
  3. Data type name

    • "counter" seems appropriate assuming it would/could be used only for that purpose?
    • From the docs links @takameyer sent, it's interesting that Kotlin and .NET opted for MutableRealmInt/RealmInteger.
nirinchev commented 3 weeks ago

Regarding the name - the reason .NET and Kotlin opted for some variation of Realm + Integer is to be aligned with other types - e.g. Realm + List is a type that behaves similarly to a BCL List, but adds some Realm API on top of it, Realm + Object is a regular object + some Realm stuff on top, so it seemed appropriate for a value that is an integer + some extra API to name it in a similar fashion. It also serves future-proofing purposes where - if necessary - we can add more functionality, that may or may not be related to counting, without breaking changes or making the API awkward.

elle-j commented 3 weeks ago

Okay yeah interesting, makes sense.

elle-j commented 2 weeks ago

P.S. looks like some first steps were taken about a year ago.