Replies: 3 comments 2 replies
-
If the ObjectManager derives from the TypeManager, I think it is OK (it makes test fixtures easier as well). If a new ObjectManager() is used somewhere (other than a test), that should be changed to pull the OM from the TypeManager. |
Beta Was this translation helpful? Give feedback.
-
If a class only needs ObjectMapper, I think it is fine to use that directly since it makes texting more succinct. Also, ObjectMapper contains more methods than TypeManager. We added some basic de/serialization to TypeManager as a convenience for not having to catch checked exceptions. I would tend to leave this decision up to specific implementations. |
Beta Was this translation helpful? Give feedback.
-
I don't think TypeManager should reproduce the methods of ObjectMapper. If it causes confusion, I'd rather remove the convenience methods that wrap common ObjectMapper methods to throw unchecked exceptions. My preference would be to not pass TypeManager to classes that do not specifically require one of its methods as doing so does not provide any value. The use of new ObjectMapper() in classes other than tests should not be done, so it would be good to correct that in a small, targeted PR. |
Beta Was this translation helpful? Give feedback.
-
I see around the codebase a lot of direct usages of the
ObjectMapper
class/instances (e.g.CrawlerJob
)For my understanding, for ser/des object from/to json
TypeManager
should be used.So, I wanted to know if are there reasons to avoid it and using
ObjectMapper
otherwise if we just need to useTypeManager
everywhere.Beta Was this translation helpful? Give feedback.
All reactions