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

Feature: :allow_nil to use in conjunction with using: SomeSerializer, and option serializes #14

Open
nicooga opened this issue Nov 6, 2017 · 5 comments

Comments

@nicooga
Copy link
Contributor

nicooga commented Nov 6, 2017

Reading tests I see that the only way to avoid an error when serializing a nested record that may not be present is using the :if option. I think is a good default for the library to raise an error when the associated record is not present. But it would also be nice to have an allow_nil or similar named option that served as a shortcut to achieve this behavior. This way we could go from writing this:

expose :post,
 using: PostEntity,
 if: fn
   (%Comment{post: %Post{}}, _opts) -> true
   (_, _) -> false
 end

To this:

expose :post, using: PostEntity, allow_nil: true

Yes I know that's a lot of of unneeded pattern matching. But I think that the more pattern match you can do the better, since you are ensuring that your serializer is not passed something it does not expect. In that sense I think it may also good idea to add a serializes SomeModule top level option to specify what struct or structs this entity can accept:

defmodule PostEntity do
  use Maru.entity
  serializes Post
  expose :id 
end
@nicooga nicooga changed the title Feature: :allow_nil to use in conjunction with using: SomeSerializer Feature: :allow_nil to use in conjunction with using: SomeSerializer, and option serializes Nov 6, 2017
@falood
Copy link
Member

falood commented Nov 6, 2017

I think allow_nil: true is useful, but I can't get the idea about serializes SomModule, if we have such code:

expose :post, using: PostEntity

defmodule PostEntity do
  serializers Post
end

what will happen when we pass %{post: %NotPostStruct{}} to it?

@nicooga
Copy link
Contributor Author

nicooga commented Nov 7, 2017

I'd expect Entity.serialize!(map) to raise an error if map is not a struct of the expected type.

defmodule Dog do
  defstruct [:color, :age]
end

defmodule DogEntity do
  use Maru.Entity
  serializes Dog
end

# Option 1, Not the "elixir way" but keeps the API the same
DogEntity.serialize(%Dog{}) # returns a serialized dog
DogEntity.serialize(%NotADog{}) # raises Maru.SerializationError

# Option 2, modifies API but I think is better
DogEntity.serialize(%Dog{}) # returns {:ok, serialized_dog}
DogEntity.serialize(%NotADog{}) # returns: {:error, :invalid_module_type}
DogEntity.serialize!(%Dog{}) # returns dog
DogEntity.serialize!(%NotADog{}) # raises Maru.SerializationError

@falood
Copy link
Member

falood commented Nov 8, 2017

Hmm, I don't think we need verify check like this.
In my use case, sometimes I need DogEntity.serialize for both %Dog{} and general map.

@nicooga
Copy link
Contributor Author

nicooga commented Nov 8, 2017

I know not anyone needs it but it would be nice to have. You could also pass a list of modules to allow multiple modules. I think we need more opinions here. Additionally, I've noticed you cannot pass a captured function to the :if option:

expose :some_attr, if: &some_func/2

... which also limits your options.

Anyway, there may be better things to work on at the moment.

@falood
Copy link
Member

falood commented Nov 8, 2017

Yes, I'm using it to parse data from mongodb, I also think I missed some important features.
Let's do them one by one, open one issue for one feature you need, and mark important ones as critical 🙂

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