Refactoring for SOC
ghola opened this issue · 3 comments
Normalizing/denormalizing (converting from object to/from array) and encoding/decoding (converting from array to/from string) are separate concerns. With the introduction of support for binary data the code inflated by quite a bit and has become had to manage. As such, I opted to follow the pattern used in the Symfony Serializer when I refactored the serializer to be used in my library. Here are the three main files that resulted:
- ObjectNormalizer deals only with transforming an object to and from an array.
- JsonEncoder deals with the actual json encoding/decoding, binary data encoding/decoding, and zero fraction support.
- JsonSerializer just uses the previous two.
In my code I stripped out the bit of functionality that deals with scalars because I'm only dealing with objects, but you get the point.
It wouldn't be a great effort to apply the same to zumba/json-serializer, but it would break BC compatibility so it would need to bump to version 3.x. You could also consider transforming this library to leverage the Symfony Serializer and just implement an encoder and a normalizer for it. This could potentially open a larger user base for it, as IMO the current normalizers used by the Symfony Serializer are useless.
What do you think?
I like the separation. When I worked on the namespace renaming (#19) was exactly to split the single class in multiple classes. Also to open for multiple normalizers (see #16 and #25).
Regarding the integration with symfony, I don't know... Seems on the 3.1 version they have a Json serializer. Since they have their own version making this library compatible will probably not add a lot of value, unless you aiming to use something that their library doesn't support. In addition, supporting Symfony's serializer interfaces could restrict some functionalities on this library. Ie, the output also needs to be compatible with Symfony. Makes sense?
Atm, the Symfony serializer package just offers a framework on which one can build it's own custom implementations. It does offer some normalizers and encoders out of the box but in my experience they offer very limited use cases. For example none of their object normalizers is fully capable of actually normalizing objects. And I'm not talking about edge cases like DateTime or magic properties ... they can't correctly handle straightforward POCOs because they rely of constructors, getters and setters instead of going straight for the class members. Why they do this is beyond me, because the most classic example of serialization is that done by an ORM, and all ORMs go directly for the class members and avoid messing with methods and constructors which can create bizarre side effects.
When searching for a serializer for the PHP Service Bus, my first stop was the JMS serializer. Then I noticed it cannot work without mappings and I deemed it too cumbersome to use. My next stop was the Symfony serializer but soon realized that it's full of bugs when dealing with objects. It was a huge disappointment to see that the most popular JSON serializers out there were actually unusable. It was only afterwards that I stumbled upon this library.
As such, my suggestion was to fill this void in the capabilities of the Symfony package while at the same time letting some of the Symfony popularity rub onto this library (more popular = more contributors = more eyes on the code = better implementation, in theory).
But I do see how some of the Symfony interfaces might not be completely suitable. For example the deserialize method requires the class type to deserialize into and this library does not require that since the serialization includes the type.
Ultimately, going the Symfony way is your decision, but I'm glad that you agree that refactoring it into smaller components would be beneficial, and I dare say that even if Symfony was not the way to go it would still be useful to heavily borrow from their approach (Encoders, Normalizers, Chains).
@ghola Splitting the class as you said makes totally sense for me. This will definitely be more extendable and maintainable.
Glad to hear that after trying multiple json serializer you considered our one of the best. 💯
Regarding Symfony support, we can treat it separately from this split. I believe we can have some sort of integration, but I haven't look deeply in Symfony's code to see how it can be done or what needs to be changed to be compatible in terms of input/output.
I created the #27 for us to track it.