Skip to content
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

Add default key support to HiveType adapters #724

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MaGaroo
Copy link
Contributor

@MaGaroo MaGaroo commented Jul 14, 2021

Hi,

@janniklind had requested(#578) a feature to add isKey field to HiveField annotation and use the field with isKey=true as default key of that class instances in add function.

So I implemented it. Before this, the add method of the base box class was using keystore.autoIncrement as default key. Now there's ability for people to set one of that class getters as default key.

For example:

@HiveType(typeId: 1)
class Dog {
    @HiveField(0, isKey: true)
    int id; 

    @HiveField(1)
    String name;

    Dog(this.id, this.name);
}

and therefore in main function:

  var dog = Dog(26, "mydog");
  box.add(dog);

It will use 26 as the key of inserted dog.

@themisir
Copy link
Contributor

themisir commented Jul 14, 2021

Thanks for your contribution 😇

However I have a bit of concern about the implementation. Wouldn't adapter lookup when adding new entries affect the speed of writes?

For example if you've putAll N entries to the box, there'll be N lookups for each item and that's not limited to just complex types (which requires custom adapters) also putting plain values like strings, numbers will also use findAdapterForValue. I think we might somehow optimize this behavior.

@MaGaroo
Copy link
Contributor Author

MaGaroo commented Jul 15, 2021

You're right.
I'll go into it in a few hours.

@themisir
Copy link
Contributor

You're right.
I'll go into it in a few hours.

Let's discuss it before implementation. Maybe it's not worth to reduce performance in favour of implementing this feature because users can extend their models from HiveObject to support key fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants