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

Add global annotation Property #6252

Closed
wants to merge 1 commit into from

Conversation

kostya
Copy link
Contributor

@kostya kostya commented Jun 24, 2018

This usefull when you want to set meta data between shards (and dont want to create annotations for every shard).
Also fix bug with yaml and json umapped usage.

This usefull when you want to set meta data between shards.
Also fix bug with yaml and json umapped usage.
@kostya
Copy link
Contributor Author

kostya commented Jun 24, 2018

specs failed, i think in 0.25.1 add only line: types["Property"] = AnnotationType.new self, self, "Property", what you think?

@bew
Copy link
Contributor

bew commented Jun 24, 2018

It's a nice idea, but Property looks way too generic to me, maybe Serialization(ignore: true) ? This way I think it's easier to see what it is about, and how it relates to JSON/YAML/you-name-it (de)serialization.

@kostya
Copy link
Contributor Author

kostya commented Jun 24, 2018

this is need not only for serialization but for many other things.

@bew
Copy link
Contributor

bew commented Jun 24, 2018

Can you give examples of non-serialization usecase?

@jhass
Copy link
Member

jhass commented Jun 24, 2018

I think we still haven't set in stone that we never will impose a specific interface on annotations. When we do decide this, this would backfire quite a bit, so I'm not sure I like it and be it for the reason that it makes it harder to later do enforce interfaces on annotations.

@asterite
Copy link
Member

Just ignore for YAML, JSON, and any other format you want. Crystal should not become a "write less oriented" language.

@kostya
Copy link
Contributor Author

kostya commented Jun 26, 2018

example: when you have many modules which generate methods based on instance_vars:

# generate doc for class
module Doc
  def to_doc
     {% for ivar in @type.instance_vars %} ... {% end %}
  end
end

# Calculate md5 based of class fields
module Md5
  def to_md5
     {% for ivar in @type.instance_vars %} ... {% end %}
  end
end

# Generate methods to_named_tuple, to_hash, ...
module To
  def to_named_tuple
     {% for ivar in @type.instance_vars %} ... {% end %}
  end

  def to_hash
     {% for ivar in @type.instance_vars %} ... {% end %}
  end
end

and others.

And you want to use it like this:

class A
   include JSON::Serializable
   include YAML::Serializable
   include MessagePack::Serializable
   include Doc
   include Md5
   include To

   @a : Int32

   @[Property(ignore: true)]
   @b : String?
end

instead of:

@[JSON::Field(ignore: true)]
@[YAML::Field(ignore: true)]
@[MessagePack::Field(ignore: true)]
@[Doc::Field(ignore: true)]
@[Md5::Field(ignore: true)]
@[To::Field(ignore: true)]
@b : String?

And another usage is modules which extend class with another variable:

module Bla
   @[Property(ignore: true)]
   @some_storage = Hash(String, String).new
end

class A
  include JSON::Serializable
  include YAML::Serializable
  include Bla
end

you dont know where this module would be included, and what ignore it should add (JSON::Field, or others.)

So shared Property would be nice in all this examples:

@asterite
Copy link
Member

@kostya The thing is, nobody will write such code (maybe only you?). When I write a REST API, I provide JSON output, not ten thousand kinds of outputs. When I parse a file with yaml configuration, I use YAML serialization. When I want to send something over a socket fast, maybe I'd use msgpack. But combining serialization just to be as generic as possible doesn't seem to have a good use case.

And even if there's a use case, just write all that code. You write a lot of code in C# and it's consistent. You write a lot of code in Elixir and it's consistent. The way to destroy consistency is by introducing hacks just because you don't want to write more.

@kostya kostya closed this Jun 26, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants