Crystal: YAML mapping initializer that must call another initializer

Created on 11 Jul 2017  路  15Comments  路  Source: crystal-lang/crystal

Hello,

I would like to initialize an instance variable which is not mapped by the YAML, and which cannot be nil. The initializers created by YAML.mapping are not calling super() or anything, is there a way to do that ? Is the standard lib should be improved ?

require "yaml"

class A
  @infos : Int32

  def initialize
    @info = 1
  end
end

class B < A
  YAML.mapping(
    a: String
  )
end
Error in test.cr:4: instance variable '@infos' of A was not initialized in all of the 'initialize' methods, rendering it nilable

Most helpful comment

You talking about prepending a call to initialize() without arguments to initialize(JSON::PullParser)? There is no way to use super as there will not be a super class in many cases.
And I don't think this is a good idea in general because it could lead to unwanted side effects. Maybe you want the regular argless initializer to do something special like fetch initial ivar values from somewhere which should not happen if it is parsed from json.

If you need some additional initialization you can just overwrite initialize(JSON::PullParser) in the specific class:
````crystal
class B < A
YAML.mapping(
a: String
)
def initialize(parser : JSON::PullParser)
super(parser)
@info = 1
end
end

All 15 comments

Duplicate of https://github.com/crystal-lang/crystal/issues/3325 but for YAML.mapping.

Oh sorry, this is slightly different, I misread.

In this case you can just assign the default value directly where you declare the instance variable @infos : Int32 = 1.

If it's more complicated you could overwrite initialize(YAML::PullParser).

Thanks for your answer.
However, it will not work if you inherit from another class which have at least one attribute (constructors are not called, ...).

require "yaml"

class A
  @infos : Int32 = 1

  def initialize
  end
end

class B < A
  YAML.mapping(
    a: String
  )
end

I read the code of this initializer before posting this issue, do you think there is a problem with adding a super() call into that one ?

Could you elaborate what is not working? I am unsure about your specific problem.

it is impossible to have a class B inheriting from A if B uses YAML.mapping and A have a non-nil instance variable.

Having a class B inherit from A as you described is certainly possible, and it is also possible to create an instance of this class. I suppose you refer to some specific use case?
Please show us what code exactly does not work and what error message the compiler gives you. Your example above compiles without error. Without any further detail I don't see a way to assist you.

I already gave it (code and error) in the first message (and in the second message i wrote a modified version of it too, the error message was the same).

Edit: Ok I must have been dizzy because it worked, my bad :) (I though it didn't worked)

Well, the error from your initial example should be gone with the modification I explained in my first comment and which you have included in your second example. This one compiles without errors (see https://carc.in/#/r/2chy), so I really can't tell what error you are facing.

Yes I though it didn't worked because I must have made a mistake when testing it, your solution is good, there are no error.

Furthermore, what about my proposition to add a super() call into the initilizer generated by YAML.mapping ?

You talking about prepending a call to initialize() without arguments to initialize(JSON::PullParser)? There is no way to use super as there will not be a super class in many cases.
And I don't think this is a good idea in general because it could lead to unwanted side effects. Maybe you want the regular argless initializer to do something special like fetch initial ivar values from somewhere which should not happen if it is parsed from json.

If you need some additional initialization you can just overwrite initialize(JSON::PullParser) in the specific class:
````crystal
class B < A
YAML.mapping(
a: String
)
def initialize(parser : JSON::PullParser)
super(parser)
@info = 1
end
end

Yes, it looks much better than my idea, thanks a lot !

Hello, I had this problem once more and I had to implement "from_json" manually.
However, I think it cool be interesting to improve the from_json() in order to be able to handle additional arguments.

Maybe an example:

require "json"

class Foo
  JSON.mapping(a: Int32)
end

class Bar < Foo
  getter b : Int32?
  def initialize(jpp : JSON::PullParser, @b = nil)
    super(jpp)
  end

  def self.from_json(io_or_string, *args)
    parser = JSON::PullParser.new io_or_string
    if args.empty?
      new parser
    else
      new parser, *args
    end
  end
end

pp Bar.from_json "{\"a\": 1}", 2
pp Bar.from_json "{\"a\": 1}"

A version of to_json with splat arguments could be useful for this case.
Maybe should I give more explanations on the problem ?

As I understand it, JSON.mapping should be used to implement a (mostly) complete serialization of an object's state to JSON and back, so in general it wouldn't need any additional args for the from_json initializer because everything should come from the JSON parser. Having a requirement for this could be a sign of bad design. There might be valid applications like perhaps dependency injection (I don't know about your usecase), but I don't think this should be promoted by the default JSON.mapping implmenetation. As you show, it is quite easy to add this yourself when needed.

Btw. you don't need the args.empty? condition - if args is empty, *args will expand to nothing:

    parser = JSON::PullParser.new io_or_string
    new parser, *args

This reopening is a duplicate of https://github.com/crystal-lang/crystal/issues/4140 I think.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

asterite picture asterite  路  139Comments

ezrast picture ezrast  路  84Comments

akzhan picture akzhan  路  67Comments

sergey-kucher picture sergey-kucher  路  66Comments

RX14 picture RX14  路  62Comments