An empty fields bug fix

[esuljic]

Currently, when a class doesn't have Hive Fields, it writes a byte in write method, but doesn't read it in the read method, which breaks the reading of the classes after that one.

I didn't write any tests for this, but I tried it in my project, and it works.

P.S. That bug is also present if there are HiveFields, but they are not in the constructor, like this:

In the screenshots above, the change is generated by this fix commit.

[Rexios80]

What is the use case for storing a class with no fields? Wouldn't that basically be an enum?

As for the second case where you don't have the fields in the constructor that also seems like something you should use an enum for.

[Rexios80]

Also if you want this merged it needs a test

[esuljic]

In this usecase, I have a class UserPreferences, that has last sorting method used as a field, which has an object that is inherited from an abstract sort class. One of its implementations is a class EmptySort, which means that they are not sorted. It doesn't have any fields, but it can't be an enum because all other types of sort have fields that are not same for all of them (its a more complex sorting algorithm).

I would say, each inheritance tree that have one non abstract class that is free of constructir fields currently have a bug with hive_ce.

I migrated from hive, and I didn't notice this bug there in production (but maybe it was there the whole time). When I looked at the implementations, only their write method was generated differently - they didn't writeByte(0), they wrote the private fields out to the writer.

Sorry for not writing a test, I wanted but couldn't find a place for this one. It would be great if you can do it.

[Rexios80]

It looks like there aren't actually any existing tests for the type adapter code generation, but the existing tests should help you formulate one for this PR. You can add a test to test/type_adapter_generator_test.dart and base the test on the generation tests in test/adapters_generator_test.dart.

[Rexios80]

Was there something wrong with the Hive PR solution of removing the if statement for not having fields? It looks like that code should work.

[esuljic]

It should work as well. That PR doesn't have a test, and is not merged there as well though.

[Rexios80]

If that solution works I prefer it since all generated adapters are the same with that change

[Rexios80]

Also with that change I think we can get away with not adding a test

[Rexios80]

I decided to take your change since the other one creates analysis issues

[esuljic]

Yeah, I guess it is better because the approach that is meant for non-empty fields will create closing ); on the next line, instead on the same, while the constructor is empty. Also, it sill works on my app after few days.

[esuljic]

Also the fact that there is already an if condition means that the creator thought it through, just forgot to either remove writeByte(0) in the write method, or put readByte(); in the read one. Do you think removing the writeByte(0) is better if there are no fields instead?

[Rexios80]

Removing the length byte would probably break things

[Rexios80]

Released in version 1.9.0-pre.2

[Rexios80]

Whoops forgot to give you credit hold on

[Rexios80]

There we go. Thank you for the contribution!

[esuljic]

Thank you for the credit @Rexios80!

[Rexios80]

It makes me feel good when other maintainers do it for me, so I like to pass on the good vibes.