pinchbv / floor

The typesafe, reactive, and lightweight SQLite abstraction for your Flutter applications
https://pinchbv.github.io/floor/
Apache License 2.0
973 stars 191 forks source link

question: What are the side effects of having an attribute of an entity as no final modifier #329

Closed cassioseffrin closed 4 years ago

cassioseffrin commented 4 years ago

I need to change a value of an attribute with his set method, this attribute could be @ignored (transient) but need to not have the final modifier.

I suspect It could cause some issues with the stream, but in my case it will not represent any changes on the stream. I will just modify him before the stream will be disposed.

So my question is: It will cause any further side effect to omit the final modifier and create a set method for an @igone(d) attribute of an entity?

cassioseffrin commented 4 years ago

and if the attribute isn't a @ignored, it is an ordinary floor attribute just not final one.

mqus commented 4 years ago

If you change anything in the entity you got from a DAO getter, nothing will happen in the database side. In fact to tell the database that something changed, you will have to take your entity and call a DAO method which has the @update annotation. We don't watch the entities we hand out. Even Streams just create a new instance every time something changes (which will then not include the changes you made locally).

There are basically 2 things floor uses and needs in an entity/view class:

What it doesn't really look at: final or other modifiers, getters, setters or any other methods, custom constructors, static fields or methods, etcpp. You can add them and floor will not care for or consider these things.

You can take a look at the generated code (the *.g.dart file(s)) yourself, it should be fairly readable.

cassioseffrin commented 4 years ago

@mqus I appreciated your clear explanation. I have already checked the *g.dart files. But nothing how to hear it from the people that have projected Floor.

The final modifier is used in flutter mainly because of the stateful widgets: StatefulWidget. But I don't think we need to follow this approach for all the dart files.

From Flutter: StatefulWidget instances themselves are immutable and store their mutable state either in separate State objects that are created by the createState method, or in objects to which that State subscribes, for example Stream or ChangeNotifier objects, to which references are stored in final fields on the StatefulWidget itself.

Thks again, I am closing this question.

cassioseffrin commented 4 years ago

Today I was asking myself about this mutation. I found I better approach for that.

Although it's working, it's not a good practice work with mutations. The better approach will be use a builder and copy the entire entity as a new one using a builder. If the entity fields were non-final, users might believe that somehow update a value in the database, this can lead to bad interpretations and so on...

I also discovered that moor has a built it copyWith method generated by the build_runner.

There is also a nice dart pub package copyable. The copyable generate the boilerplate for copyWith. But unfortunately the copyable cause some conflicts with the analyzer used in floor.

So, what do you think about implement a new feature to generate the builder copyWith in floor?

Best regards!

mqus commented 4 years ago

I also discovered that moor has a built it copyWith method generated by the build_runner.

Thats because moor also generates the code for the entities. We only generate the Dao Code and the CREATE TABLE statements (i glossed over some details but that's mostly it).

So, what do you think about implement a new feature to generate the builder copyWith in floor?

Could you provide a code example on how (and where) you would use that together with the existing features? I can imagine that we may be in a position to solve the following one:

But unfortunately the copyable cause some conflicts with the analyzer used in floor.

But I can't think of a good way to integrate copyWith in our existing concept.

cassioseffrin commented 4 years ago

My ideia was something like what flutter have made in: Serializing JSON

take a look at Serializing JSON using code generation libraries.

@JsonSerializable()
class Address {
  String street;
  String city;
  Address(this.street, this.city);
  factory Address.fromJson(Map<String, dynamic> json) => _$AddressFromJson(json);
  Map<String, dynamic> toJson() => _$AddressToJson(this);
}

Others frameworks like hibernate (Java) treats the persistence entities attached to database, so when you change a mutable entity it will reflect the changes in database. When you don't want the reflections to be propagated in database it's necessary to detach the entity. That's another reason to keep the attributes in floor entity with the final modifier. When changes are needed copy the object as new one with the necessary modifications, therefore keeping it immutable.

To be more clear what I am suggesting is to implement an existing copyWith factory pattern to floor. But if it will take too much efforts, may we can figure out how to solve the conflicts with the analyzer.

mqus commented 4 years ago

Yeah I see where you are coming from but this is not the lightweight approach from floor or room at all, we do not do magic things implicitly but always explicitly. If you want to update your entity in the db, use the update function of your dao. Floor/Room is mostly a thin typed layer over the actual database interactions, not a full-fletched ORM, which means that you can use copyWith, but that is nothing we have to provide and is better solved with an extra package.

mqus commented 4 years ago

And tbh I probably still didn't understand your request clearly, as your code example neither has a copyWith method, nor any existing database interactions with floor, which is what I (perhaps not clearly) asked for.

Your Code example is 100% possible today in floor, because it does not interact with it. I want to see an example of you using the feature as you want it to look like when interacting with the database. I can't follow comparisons with other libraries well because they are not structured the same and have completely different requirements.

cassioseffrin commented 4 years ago

Your Code example is 100% possible today in floor, because it does not interact with it. I want to see an example of you using the feature as you want it to look like when interacting with the database. I can't follow comparisons with other libraries well because they are not structured the same and have completely different requirements. I know that, and it's working pretty well. I am just suggesting an improvement. Look what they do in moor, may you will get my point.

For now it's ok the way it is. I will control that data flow by my own implementations, probably with https://pub.dev/packages/copyable. I would've done it if it wasn't the conflicts, also we need to avoid use a tons of 3th party libraries when it's possible.

It's was a nice discussion over the topic.. Thks (it's closed already)!

mqus commented 4 years ago

Look what they do in moor,

Moor is a very different (and from my perspective much more complex) beast conceptually, even if the naming is somewhat similar. I didn't start the floor project but in my mind floor is striving to become flutters equivalent to Androids room and not JPA/Hibernate. That said, I'm currently working on improving and integrating moors sqlanalyzer package into floor (don't expect results anytime soon :wink:), so there is definitely functionality we can borrow.

On another note, I don't see why you could not use copy_with or copy_with_extension as those seem to be compatible (1, 2) with floors version of analyzer. Please open a new issue if you can't get it to work, because then it should be a bug in floor and/or copy_with(_extension)

cassioseffrin commented 4 years ago
On another note, I don't see why you could not use copy_with or copy_with_extension as those seem to be compatible (1, 2) with floors version of analyzer.
Please open a new issue if you can't get it to work, because then it should be a bug in floor and/or copy_with(_extension)

I could not agree more! I consider floor much better than moor. And may you have noticed I am rigorous in design patterns. I have tested at least 5 alternatives and I really believe in the future of floor. For your surprise in comparison with hibernate we have jaguar_orm followed by floor (2nd place).

I haven't tried copy_with or copy_with_extension yet. My first try was copyable, but as I said copyable caused some conflicts with floor, so I decide to abandon "copyable". I have tested copyable first because it's best positioned in github than copy_with and copy_with_extension (both very new extensions).

mqus commented 4 years ago

Ah right I didn't check which package you referred to, copyable was last updated almost a year ago and is not really compatible with floors version of analyzer...