Multiple serializers #66

nulldef · 2018-02-01T21:33:06Z

Resolve #65

houndci-bot · 2018-02-01T21:33:15Z

lib/surrealist/vars_helper.rb

-        self_class.instance_variable_set(SERIALIZER_CLASS, serializer_class)
+      # @param [Symbol] tag a tag associated with serializer
+      def add_serializer(self_class, serializer_class, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:15Z

lib/surrealist/vars_helper.rb

-      def find_serializer(klass)
-        klass.instance_variable_get(SERIALIZER_CLASS)
+      def find_serializer(klass, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:15Z

lib/surrealist/instance_methods.rb

@@ -49,7 +49,8 @@ module InstanceMethods
    #   # => "{\"name\":\"Nikita\",\"age\":23}"
    #   # For more examples see README
    def surrealize(**args)
-      if (serializer = Surrealist::VarsHelper.find_serializer(self.class))
+      serializer = Surrealist::VarsHelper.find_serializer(self.class, tag: args[:tag])
+      if serializer


Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

houndci-bot · 2018-02-01T21:33:15Z

lib/surrealist/vars_helper.rb

-        self_class.instance_variable_set(SERIALIZER_CLASS, serializer_class)
+      # @param [Symbol] tag a tag associated with serializer
+      def add_serializer(self_class, serializer_class, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:16Z

lib/surrealist/vars_helper.rb

-      def find_serializer(klass)
-        klass.instance_variable_get(SERIALIZER_CLASS)
+      def find_serializer(klass, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:16Z

lib/surrealist/instance_methods.rb

@@ -49,7 +49,8 @@ module InstanceMethods
    #   # => "{\"name\":\"Nikita\",\"age\":23}"
    #   # For more examples see README
    def surrealize(**args)
-      if (serializer = Surrealist::VarsHelper.find_serializer(self.class))
+      serializer = Surrealist::VarsHelper.find_serializer(self.class, tag: args[:tag])
+      if serializer


Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

houndci-bot · 2018-02-01T21:33:16Z

lib/surrealist/vars_helper.rb

-        self_class.instance_variable_set(SERIALIZER_CLASS, serializer_class)
+      # @param [Symbol] tag a tag associated with serializer
+      def add_serializer(self_class, serializer_class, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:16Z

lib/surrealist/vars_helper.rb

-      def find_serializer(klass)
-        klass.instance_variable_get(SERIALIZER_CLASS)
+      def find_serializer(klass, tag: nil)
+        tag = tag || DEFAULT_TAG


Use self-assignment shorthand ||=.

houndci-bot · 2018-02-01T21:33:16Z

lib/surrealist/instance_methods.rb

@@ -49,7 +49,8 @@ module InstanceMethods
    #   # => "{\"name\":\"Nikita\",\"age\":23}"
    #   # For more examples see README
    def surrealize(**args)
-      if (serializer = Surrealist::VarsHelper.find_serializer(self.class))
+      serializer = Surrealist::VarsHelper.find_serializer(self.class, tag: args[:tag])
+      if serializer


Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

nesaulov

Could you please also add some specs on this new feature? (covering both #surrealize & #build_schema)

nesaulov · 2018-02-03T13:57:13Z

README.md

@@ -5,7 +5,7 @@
 [![Gem Version](https://badge.fury.io/rb/surrealist.svg)](https://rubygems.org/gems/surrealist)

 ![Surrealist](surrealist-icon.png)
- 
+


Could you please rollback changes concerning whitespaces here? They are necessary for proper code displaying.

nesaulov · 2018-02-03T13:58:21Z

lib/surrealist.rb

@@ -148,7 +148,8 @@ def build_schema(instance:, **args)
    #   provided in the object's class. Values will be taken from the return values
    #   of appropriate methods from the object.
    def __build_schema(object, **args)
-      if (serializer = Surrealist::VarsHelper.find_serializer(object.class))
+      serializer = Surrealist::VarsHelper.find_serializer(object.class, tag: args[:tag])
+      if serializer


I think you forgot to apply the new feature to the #build_schema method.

There is no external serializer support for this method

Yeah, I guess I missed it in #61.

nesaulov · 2018-02-03T13:59:40Z

lib/surrealist/class_methods.rb

@@ -98,9 +98,9 @@ def delegate_surrealization_to(klass)
    # @param [Class] klass a class that should inherit form Surrealist::Serializer
    #
    # @raise ArgumentError if Surrealist::Serializer is not found in the ancestors chain
-    def surrealize_with(klass)
+    def surrealize_with(klass, tag: :default)


Maybe we should use DEFAULT_TAG here also?

nesaulov · 2018-02-03T14:00:25Z

lib/surrealist/vars_helper.rb

-        klass.instance_variable_get(SERIALIZER_CLASS)
+      def find_serializer(klass, tag: nil)
+        tag ||= DEFAULT_TAG
+        klass.instance_variable_get(SERIALIZER_CLASSES).try(:[], tag.to_sym)


We don't have ActiveSupport in here 🙃

You can use return + fetch(tag.to_sym, nil) here I guess (safe navigation would be better, but it's not available in ruby 2.2)

nesaulov · 2018-02-03T15:38:03Z

lib/surrealist.rb

@@ -148,7 +148,8 @@ def build_schema(instance:, **args)
    #   provided in the object's class. Values will be taken from the return values
    #   of appropriate methods from the object.
    def __build_schema(object, **args)
-      if (serializer = Surrealist::VarsHelper.find_serializer(object.class))
+      serializer = Surrealist::VarsHelper.find_serializer(object.class, tag: args[:tag])
+      if serializer


Yeah, I guess I missed it in #61.

nesaulov · 2018-02-03T15:39:03Z

lib/surrealist/vars_helper.rb

+      def find_serializer(klass, tag: nil)
+        tag ||= DEFAULT_TAG
+        hash = klass.instance_variable_get(SERIALIZER_CLASSES)
+        return if hash.nil?


Maybe hash && hash.fetch() ?

nesaulov · 2018-02-03T15:42:33Z

spec/multiple_serializers_spec.rb

+      end
+
+      it do
+        json = Surrealist.surrealize_collection(collection, tag: :short)


Maybe extract this to rspec's let, so we can make it a one-liner?
Or use specify instead of it

nesaulov · 2018-02-03T17:59:09Z

@nulldef one more thing: we need an error message for the case when the wrong tag is passed. From your spec right now:

Surrealist.surrealize_collection([post1, post2], tag: :kek)
  Surrealist::UnknownSchemaError: Can't serialize Post - no schema was provided.
from /Users/nesaulov/Code/ruby/opensource/surrealist/lib/surrealist/exception_raiser.rb:49:in `raise_unknown_schema!'

The error message is wrong and misleading. I suggest something like "The tag specified (#{tag}) has no corresponding serializer".

nesaulov · 2018-02-04T18:08:05Z

Thank you, @nulldef! 🎉

nulldef added 2 commits February 1, 2018 21:06

ok

ba6734b

readme & specs

67c68a8

houndci-bot reviewed Feb 1, 2018

View reviewed changes

nulldef added 2 commits February 2, 2018 00:35

fix style

417f9ba

fix

279f0ca

nesaulov requested changes Feb 3, 2018

View reviewed changes

fixes

1bcfc17

nulldef force-pushed the multiserializer branch from 923c13c to 1bcfc17 Compare February 3, 2018 15:30

nesaulov reviewed Feb 3, 2018

View reviewed changes

fixes

a39f864

fix

d124c55

nesaulov approved these changes Feb 4, 2018

View reviewed changes

nesaulov merged commit 70dcb5f into nesaulov:master Feb 4, 2018

nulldef deleted the multiserializer branch February 27, 2018 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple serializers #66

Multiple serializers #66

nulldef commented Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

houndci-bot Feb 1, 2018

nesaulov left a comment

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018 •

edited

Loading

nulldef Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov Feb 3, 2018

nesaulov commented Feb 3, 2018

nesaulov commented Feb 4, 2018

		@@ -5,7 +5,7 @@
		[![Gem Version](https://badge.fury.io/rb/surrealist.svg)](https://rubygems.org/gems/surrealist)

		![Surrealist](surrealist-icon.png)

Multiple serializers #66

Multiple serializers #66

Conversation

nulldef commented Feb 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nesaulov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nesaulov Feb 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nesaulov commented Feb 3, 2018

nesaulov commented Feb 4, 2018

nesaulov Feb 3, 2018 •

edited

Loading