-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[dart] Expose internal variables for ObjectBuilder type #8839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@vaind mind taking a peek at this PR and leaving some thoughts? |
I won't have time to do so for a while but will have a look when I do and this is still open. |
|
@vaind I actually hope to make many incompatible modifications, such as using Object Builder as a separate object model (but without direct builder functionality), and adding serialization, deserialization, and other functions to this object model. This PR is the smallest modification made on the basis of compatibility, and I am actually not very satisfied with it. |
|
@fawdlstty given
I'm not super excited about reviewing this in it's current state if this is true. I know very little/nothing about Dart, so I would have to lean on others to know if this is a feature set that is beneficial to all of the flatbuffers dart community or not. |
vaind
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a look. This is exposing internal (private) members, breaking encapsulation. As such, I would not recommend merging (I can't approve/request-request changes here directly because I'm not a maintainer).
|
The effect I need to achieve is to deserialize the JSON object generated by the Rust language, which has a one-to-one correspondence with the FBS object model, into the FBS object model. Then, read some of the variable values within it. Now, the object model generated by flatc has numerous issues that prevent it from meeting my requirements. @vaint Additionally, I would like to clarify that I did not violate encapsulation. I merely made the current code more user-friendly. (If you disagree, you can provide your feedback and explain how the above code usage could lead to bugs.) @jtdavis777 What I mean is not that the aforementioned pr is worthless. I hope to gradually make small adjustments within a reasonable range, so that the outcome becomes increasingly satisfactory (rather than making a huge change all at once). |
|
@vaind It would be even better if you could offer some suggestions for modifications (such as what should be done to meet my requirements). |
I haven't used flatbuffers for a while so maybe this has changed but AFAIR runtime JSON de/serialization is a non-objective of this library. You can convert JSON to flatbuffers via a command-line utility if you have preexisting JSON you'd like to use. Then you can use flatbuffer in any language interchangeably. Having a serializer and deserializer at runtime defeats the whole purpose of Flatbuffers https://flatbuffers.dev/ - e.g. "Access to serialized data without parsing/unpacking". I understand this makes sense in your usecase, it just might not be a good fit to upstream these changes to the generally available library. Which is perfectly normal in OS. |
@vaind Thank you for your reply. If I change it to a pure object model (instead of inheriting from fb.ObjectBuilder) and make the members accessible to users, will that alleviate your concerns? |
Yes, those are not the goals of this library. So for developers like me who have special requirements, using it is quite uncomfortable. In addition, FlatBuffers is not completely irrelevant to my requirements, but it has fulfilled half of them. Some parts I can use directly, while others require continuous improvement. I don't think I'm the only one who has encountered this requirement (because FlatBuffers already has a large amount of code that fulfills my needs). I hope to improve it. |
|
@jtdavis777 I don't want to spend most of my energy contributing code arguing with people who are ideologically opposed (they can easily say 'no' to features they don't need without considering the consequences). I hope to have accurate information. Have flatbuffers considered optimizing the use of this part? |
|
I have not sat down to thoroughly review this PR or the discourse yet -- it is on my list but I'm a volunteer contributing in my spare time. I will take a look when I can! |
This submission is made to fulfill one of my requirements. I have a JSON data that is exactly the same as the current FlatBuffers definition. I need to perform serialization and deserialization (at least when adding members to the FlatBuffers structure definition, I want my deserialization code to throw an error to alert me to add the corresponding member). Observation shows that ObjectBuilder is quite close to my requirements, but it hides the member variables. Therefore, I made its member variables public.