dumptruckman/PluginBase

Change Deserializer code again!

SupaHam opened this issue · 0 comments

The introduction of SerializerSet added a lot more control for users, which was great. But I find myself seeing more ways to improve on the whole deserialization process as a whole, serialization will always be the easier part.

The idea

Simply, injection. The safe kind. Currently if I wanted to give a Serializer some context to it's owner I need to do the following:

  1. Create a SerializerSet Builder instance that possibly copies off some other SerializerSet, because of the immutability.
  2. Construct my custom Serializer implementation that has a constructor of some sort that takes an instance to provide context at a later time .
  3. Add the constructed instance to the Builder.
  4. Build and store it somewhere.

That's somewhat good and free... in some ways. However, there are some possible limitations to how far this can go. For example, if I wanted to access some data, I have to execute step 2 from the list above. The data I have to pass in the constructor might be short lived, thus forcing the user to have to wrap short lived objects in a long lived, permanent object that is provided at all times.

It is not the worst case a coder could come to, but it can be improved!

Solution

As mentioned before the idea of creating a better deserialization process is dependency injection. Although there are quite a few libraries out there, Guice is the only one I've had the pleasure of messing around with. Needless to say I was impressed with it's functionality and ease-of-use.

It works a little something like this:

  • Serializer has a field annotated with @Inject.
  • Guice sees that Field and checks its type.
  • Guice checks SerializableConfig handler for any instances for the Field's type in the current deserialization process.
  • Sets if type is present, otherwise does nothing.

Example code:

public class MyFilterSerializer implements Serializer<Map<Filter, Double>> {
    ...
    @Inject
    private FilterManager manager;

    @Override
    public Map<Filter, Double> deserialize(Object serialized, Class wantedClass, SerializerSet serializerSet) throws IllegalArgumentException {
        Map map = (Map) serialized;
        for (Object key : map) {
            Filter filter = this.manager.getFilterById(key);
            ...
        }
        ...
    }
}

To break down the code above:

  1. We implement Serializer as per usual.
  2. We create a field of type FilterManager. A FieldManager contains a collection of Filter objects indexed by Strings.
  3. Implement deserialize method which makes use of the manager field. No NPE will be thrown because the injector will have already handled the @Inject field when SerializableConfig constructs MyFilterSerializer itself.

If I wanted an instance of a serializer I could probably get it through SerializerSet, but I feel there is much downtime that could be cut out by using dependency injection. Users get the data they need precisely when they ask for it in their Serializer. As opposed to having to control that themselves. Regardless, I think the current SerializerSet should remain unchanged in the hopes that users may still have control over their Serializers whenever they wish.

One thing I would hate is users wondering why they're getting NPEs. The simple solution to this would be "Is it annotated with @Inject", and I predict most beginner users will be baffled by the idea of annotations and how they are required for this functionality. I feel it might be possible that SC write it's own middleman that automatically handles Serializer fields seeing as a Serializer must be stateless. A case where a Serializer is not stateless is if the user creates their own Serializer instance and registers it to the SerializerSet, which in that case SerializableConfig would must likely be avoiding anyways.

Pros:

  • Less downtime via injection.
  • Users don't have to handle constructing certain Serializers just to give it data.
  • No more wrappers for short-lived data.
  • Control in general is still available if the user wishes to bypass dependency injection.

Cons:

  • Having to rely on a dependency injection software if SC doesn't write its own. I highly recommend SC does NOT write its own as it is unnecessary and libraries out their cover most use cases.
  • Increasing SerializableConfig footprint by at least a few hundred KBs because of having to shade a library like Guice. I don't believe this is an issue for Sponge platform as they use Guice in runtime.
  • Having to annotate all fields with @Inject.