jaguar_orm icon indicating copy to clipboard operation
jaguar_orm copied to clipboard

Immutability support relations

Open jaumard opened this issue 5 years ago • 16 comments

If model are immutable, all generated code about relations doesn't compile anymore.

Mostly because if generate stuff like this:

void associateCart(CartItem child, Cart parent) {
    child.cartId = parent.id;
  }
Future<Cart> preload(Cart model, {bool cascade: false}) async {
    model.items = await cartItemBean.findByCart(model.id,
        preload: cascade, cascade: cascade);
    return model;
  }

  Future<List<Cart>> preloadAll(List<Cart> models,
      {bool cascade: false}) async {
    models.forEach((Cart model) => model.items ??= []);
    await OneToXHelper.preloadAll<Cart, CartItem>(
        models,
        (Cart model) => [model.id],
        cartItemBean.findByCartList,
        (CartItem model) => [model.cartId],
        (Cart model, CartItem child) => model.items.add(child),
        cascade: cascade);
    return models;
  }

as models are immutable it's not possible to access setters directly

jaumard avatar Sep 25 '18 09:09 jaumard

This will not be supported.

tejainece avatar Jun 30 '19 18:06 tejainece

:( si you can't say #42 is fixed ^^ Why don't you want to support this?

jaumard avatar Jul 01 '19 05:07 jaumard

Jaguar supports immutable fields. Not immutable models. One can have immutable models if there are no relations.

If there are relations, we have to set fields to associate them.

If we come up with a method to achieve this, we can implement it.

tejainece avatar Jul 01 '19 08:07 tejainece

Yes immutable fields are supported since a long time, I already use them. But what I'm interested in is immutable models :)

There is a simple way to achieve this by just returning a full copy of object with the relations instead of settings fields. That's how the all flutter frameworks works, you have a copy method you can call with the possibility to override any fields, that method return a new immutable version of the object. What do you think about that ?

jaumard avatar Jul 01 '19 08:07 jaumard

As it's a breaking changes I think v4 is the perfect time for doing it ^^

jaumard avatar Jul 01 '19 08:07 jaumard

I think in general immutable models are awesome. But I do not agree with making the api and generation complex to get complete immutability. I have never used build_value and its shebang.

Please present a fully implemented, hand written bean example for one-to-one scenario with immutable relations. If it is does not complicate things, let us absorb it into the generator.

What say?

tejainece avatar Jul 01 '19 09:07 tejainece

I don't think we need build_value (never used it too ^^), we just need to generate a copy method on our generator, and change the preload methods to return a new object.

I'll create a one-to-one example ASAP ! I'm reopening the issue for now, if we finally don't do it we'll close it again. But I really think we can do it without to much trouble

jaumard avatar Jul 01 '19 09:07 jaumard

Sounds good.

tejainece avatar Jul 01 '19 10:07 tejainece

Here is my manual implementation of one-to-one association with immutability, all modifications can be done under the generator I think:

https://gist.github.com/jaumard/5e777dbd1f2090a7d859061aa2165f5d

Tested on my Mac with PostgreSQL DB

Let me know what you think @tejainece !

jaumard avatar Jul 01 '19 14:07 jaumard

@tejainece any news on this ? I think it will be a nice addition to v4 :)

jaumard avatar Jul 30 '19 07:07 jaumard

I have not got a clean proposal that does not sacrifice user friendliness for immutability.

tejainece avatar Jul 30 '19 08:07 tejainece

Did you check my gist ? It doesn't sacrifice anything, just give you immutability

jaumard avatar Jul 30 '19 08:07 jaumard

The approach you suggested only supports immutable models. It does not support mutable models.

tejainece avatar Aug 08 '19 08:08 tejainece

I think we should handle immutable models and mutable models differently.

tejainece avatar Aug 08 '19 08:08 tejainece

In fact it does, if you don't put your fields as final they are mutable for the users. What the generated code use is not really a concern imho. Also supporting mutable models it's supporting a bad practice... So not sure it worth supporting both differently (more work, less easy for users and more error prone)

jaumard avatar Aug 08 '19 08:08 jaumard

If you really want to support both we can keep the current way, add a boolean field on the Bean annotation and do the immutable way if that flag is true (I would push for immutable by default but if you don't want it's fine for me ^^)

jaumard avatar Aug 08 '19 08:08 jaumard