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

WIP Improve serialization format #44

Closed
wants to merge 2 commits into from

Conversation

pedrovgs
Copy link
Owner

📌 References

🎩 What is the goal?

Improve our custom serialization format by adding some random spaces. I'm sending this PR so we have something to discuss. I'd like to resolve the format and serialization issue but this PR is not the way to go. If I were you I'd not merge this branch

How is it being implemented?

We've added some spaces based on the depth level and thanks to that we can now build trees improving the format. The solution is not ideal at all because our format doesn't let us tokenize easily.

At this point, I'm thinking the best solution is to move to Gson and create a custom serialization process to transform any instance into a String and then use any library to pretty print the result. I know we decided to use toString method because of the first implementation of the library and because we don't need to deserialize any value, but using json could be really interesting. What do you think guys?

How can it be tested?

🤖

@tonilopezmr
Copy link

tonilopezmr commented Sep 24, 2018

Even using JSON or toString or whatever, I think we should still let the users change the Serialization, do you think so?

The problem with JSON (Default JSON) is we are going to miss kotlin object and also the object types.

  1. Kotlin class Developer(name="Serch") to JSON is { "name": "Serch" }
  2. Anobject WhateverError is {} in JSON.
  3. Kotlin Pair values to JSON.
  4. Add JSON parser for dates.

We could add a JSON library and customize that cases for the default serialization module. I think we lose information with JSON and we are going to add it or miss that information, it shouldn't be two different classes with the same implementation, maybe It's not useful to show the class type (except kotlin object).

@pedrovgs
Copy link
Owner Author

pedrovgs commented Sep 26, 2018

@tonilopezmr I'm going to answer your questions:

Even using JSON or toString or whatever, I think we should still let the users change the Serialization, do you think so?

We will keep this feature. I think I didn't say I want to delete this.

The problem with JSON (Default JSON) is we are going to miss kotlin object and also the object types.

As you mention, we will have to improve the default Gson lib serialization format adding support for sealed classes, for example.

I'd go for json because is a format every developer should know and we have a lot of libraries we can use to pretty print or format the json content.

What do you think guys? If you think we could give it a try I could send another PR with the GSON implementation so we can compare the implementation. When implementing the json version the key to success is to remember that we don't need to deserialize information

In the worst scenario, we already have this version ready 😃

@Serchinastico
Copy link

What about mixing both approaches? We can use JSON adding a bunch of "private" (__privatefield__ sort of thing) meta fields to every object with the name of the type.

I'd go for json because is a format every developer should know and we have a lot of libraries we can use to pretty print or format the json content.

This is the most convincing argument. Doing anything else would be reinventing the wheel.

@pedrovgs
Copy link
Owner Author

pedrovgs commented Oct 3, 2018

@Serchinastico that was my idea. Using gson and customizing the serialization for some special scenarios. I'll give it a try in a future iteration and I'll send another PR so we can easily compare both approaches and measure the impact of the change.

About private changes, I think we should not serialize them. Look at #43. Private class attributes are used just internally in Kotlin and we should try to stay as far as possible of the implementation details, even when this testing strategy is not that far xD

@tonilopezmr
Copy link

tonilopezmr commented Oct 3, 2018

I had the same idea @Serchinastico 👏for support object's and more cases. I think like @pedrovgs to not serialize private variables 👍

@Serchinastico
Copy link

By private I meant our own fields in the json object representing meta information about the object itself such as the name of the type or any other characteristic we want to store. Not private fields or anything, sorry about the confusion 😅

@tonilopezmr
Copy link

aaahh I know what you say it, that is what I said, for example with Kotlin serialization you have the information of the object type for example, Dog(name...

@pedrovgs
Copy link
Owner Author

pedrovgs commented Oct 8, 2018

I've sent another PR that aims to replace this one.

@pedrovgs
Copy link
Owner Author

pedrovgs commented Nov 7, 2018

Closing this PR in favor of #46

@pedrovgs pedrovgs closed this Nov 7, 2018
@pedrovgs pedrovgs deleted the improve-serialization-format branch November 7, 2018 10:02
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.

None yet

3 participants