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

Clean up Serializer interface and usage. #26

Open
SupaHam opened this issue Feb 28, 2016 · 0 comments
Open

Clean up Serializer interface and usage. #26

SupaHam opened this issue Feb 28, 2016 · 0 comments

Comments

@SupaHam
Copy link
Contributor

SupaHam commented Feb 28, 2016

Introduction

The Serializer class has two relevant methods currently, serialize(Object, SerializerSet) and deserialize(Object, Class, SerializerSet). Very useful, yet rather obnoxious at times. Generating a java class for creating a Serializer looks a little something like:

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public class MySerializer implements Serializer<MyObject> {
    @Nullable
    public Object serialize(@Nullable MyObject object, @NotNull SerializerSet serializerSet) throws IllegalArgumentException {
        // Serialization
    }

    @Nullable
    public MyObject deserialize(@Nullable Object serialized, @NotNull Class wantedType, @NotNull SerializerSet serializerSet) throws IllegalArgumentException {
        // deserialization
    }
}

That's not too bad without the code. Maybe remove some annotations:

public class MySerializer implements Serializer<MyObject> {
    public Object serialize(MyObject object, SerializerSet serializerSet) throws IllegalArgumentException {
        // Serialization
    }

    public MyObject deserialize(Object serialized, Class wantedType, SerializerSet serializerSet) throws IllegalArgumentException {
        // deserialization
    }
}

Still not as good as it can be. First of all wantedType, very useful for more general serializers, however it isn't used in a lot of other serializers, as they are more direct in what they expect to handle.

Proposal

With the recent restructure and overhaul of the SerializableConfig module came SerializerSet, very powerful tool that gives a lot more context to serialization. However, similar to wantedType, it isn't used as much as it is worth keeping in the method signature.

What I propose is a wrapper class that includes everything relevant to the task and cutting down both methods to one parameter.

The end product would look a little something like:

public class MySerializer implements Serializer<MyObject> {
    public Object serialize(ObjectWrapper data) {
        // Serialization
    }

    public MyObject deserialize(SerializedData data) {
        // deserialization
    }
}

Firstly, the ObjectWrapper class is one which contains an object (of type MyObject) to be serialized, which is accessible via a getter method such as getData(). At that point in time, getData() would return an Object of type Object, but it might be better if ObjectWrapper takes a generic type parameter that is MyObject in order for getData() to return the casted type immediately.

Secondly, the SerializedData class would contain two unique publicly accessible objects, the serialized data, preferably accessible via getData() for consistency. getData() would return type Object and nothing more as we can not possibly identify serialized data without causing issues in different. The second piece of data accessible via SerializedData would be getWantedClass() which would feature the current wantedClass.

It is expected that ObjectWrapper and SerializedData would be extending some sort of type that would have common data, e.g. SerializerSet.

A little less important, but also beneficial for consistency is changing the throws declaration to throw Exception. The reason for this is that any exception can (and should!) be thrown, despite IllegalArgumentException being a common go-to. Alternatively, maybe remove the throws declaration completely and just keep the javadoc lying around.

Lastly, I think we need to focus on scaring people away from using serialize and deserialize loosely and instead guiding them towards using SerializeableConfig class for usage. This way we can truly support the users in fixing issues if SerializableConfig is ever to do more than just be delegation for calling the Serializer methods. I believe this issue will be the first step towards that, in driving people away from wanting to construct the wrapper classes and dealing with that stuff themselves.

Conclusion

To conclude, this issue proposes that wrapper classes ObjectWrapper and SerializedData be created and used in Serializer. SerializableConfig can provide a lot more convenience for data access when deserializing via the provided SerializedData instance. Also proposes that the throws declaration be modified or removed completely from the serialize and deserialize methods.


Originally I was thinking incorporating #25 into this would be very beneficial, but I think the proposed wrapper classes are much more convenient and strict in always ensuring the user is guided through their usage via getters as opposed to dependency injection.

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

No branches or pull requests

1 participant