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

Serialize from hash #96

Closed
glaucocustodio opened this issue Apr 20, 2018 · 5 comments
Closed

Serialize from hash #96

glaucocustodio opened this issue Apr 20, 2018 · 5 comments
Assignees

Comments

@glaucocustodio
Copy link
Contributor

Suppose I have:

class Foo
  include Surrealist
  json_schema do
    { foo: String }
  end
end

I can't serialize passing a hash (because the gem expect methods):

Foo.new({ foo: "bar" }).surrealize # crash!

Do you plan add support for hash? I've created the thin wrapper below and it works:

class HashBased
  include Surrealist

  attr_reader :hash

  def initialize(hash = {})
    @hash = hash.deep_symbolize_keys
  end

  def method_missing(method_name, *args)
    if hash.key?(method_name)
      hash[method_name]
    else
      super
    end
  end

  def respond_to_missing?(method_name, _include_private = false)
    hash.key?(method_name)
  end
end
class Foo < HashBased
  json_schema do
    # same as before
  end
end
Foo.new({ foo: "bar" }).surrealize # now it works!

With a simple piece of code like that we can open a bunch of new possibilities.

@nesaulov
Copy link
Owner

@glaucocustodio, thank you, I guess that's a good feature to have, I would be glad accepting a PR. The only thing to keep in mind is that we don't want to pollute the Surrealist::Builder class, as it's already quite complex. I suggest having a separate class, a HashBuilder for example, which accepts only hashes and validates them directly, not based on method_missing. There will also be a different (and easier) way to validate hash schema, something like

hash.keys.length == schema.keys.length
hash.keys.map(&:to_sym).sort == schema.keys.sort

And we will also need tests covering different cases, apparently.

@glaucocustodio
Copy link
Contributor Author

Using my wrapper for hash, I just needed to change the check_for_ar method like below to support hash validation:

def check_for_ar(schema, instance, key, value)
  if ar_collection?(instance, key)
    construct_collection(schema, instance, key, value)
  else
    ins = if instance.respond_to?(key)
        instance.send(key)
      elsif instance.is_a?(Hash)
        instance[key]
      else
        instance
      end
    call(value, ins)
  end
end

@nesaulov
Copy link
Owner

You see, I don't think that 4 if statements in one method is a good thing at all. And the semantics are broken here - why would we check for active record if we are just serializing a hash

@glaucocustodio
Copy link
Contributor Author

You are right, this code is not very good. I need to take some time to create the HashBuilder.

@nesaulov
Copy link
Owner

Closing this due to #106

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

2 participants