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

Support for Lists, Sets and Arrays #49

Open
Cilenco opened this issue Sep 3, 2018 · 4 comments
Open

Support for Lists, Sets and Arrays #49

Cilenco opened this issue Sep 3, 2018 · 4 comments

Comments

@Cilenco
Copy link

Cilenco commented Sep 3, 2018

Are there any plans to add support for Lists, Set and / or arrays? Would be a nice additional feature for this library. Would you accept a pull request if I would implement this?

From a quick overview from the code I think only the Serializer and their SerializationStrategy needs to be reimplemented for this so it shouldn't be too hard.

@iamironz
Copy link
Contributor

iamironz commented Sep 4, 2018

Hi !
What kind of generic types for collections do you mean? Because you just can't serialize all data types, only primitives and Strings.
Of course, you can rewrite StringSetSerializer and Preferences classes for accepting all inheritors of Iterable interface but you should refactor serializer class for accepting all kinds of generics like primitives and Strings. All this steps requires mid codebase refactoring and if you desire create this functionality you should provide small reference proof-of-concept of your ideas.

--

Cheers,
Alex

@Cilenco
Copy link
Author

Cilenco commented Sep 4, 2018

I thought of collections of all primitive types, Strings and inheritors of Persistable.

To only rewrite the StringSetSerializer is not a good solution in my opinion. That would imply that each value is first converted to a string which may not be good practice. I think that rewriting all serializer classes is the better way to go here. I forked this repo and did some refactoring.

For POC look for example at the BooleanSerializer which now inherits from AbstractSerializer. There is most of the code for the Collection serialization. This new implementation does not break with the current one (except of FLAGS and NULL values) and also new test cases work pretty well.

Of course the Preferences as well as the PreferencesEditor interface have to change to accept collections and arrays but that shouldn't be the problem I guess.

@iamironz
Copy link
Contributor

iamironz commented Sep 5, 2018

That would imply that each value is first converted to a string

nope, I meant that you should rewrite this one for working with generic types natively not String.valueOf

inherits from AbstractSerializer

This is not good enough because your base serializer has methods which is useless for most serializers and used not always, I prefer use delegates instead of inheritors, keep your code simple.

@Cilenco
Copy link
Author

Cilenco commented Sep 5, 2018

Let me check if I got your point right:

Your idea is to only rewrite the StringSetSerializer (apart from Preferences) to accept all kinds of Collection. Then each Collection is serialized similar to a Set<String> and only the encoding of the types change? So e.g. List<Integer> would call a method in StringSetSerializer but the encoding of the elements itself is then handeled by IntegerSerializer?

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

No branches or pull requests

2 participants