concord icon indicating copy to clipboard operation
concord copied to clipboard

Generate proper initializers for concord infected inheritance trees

Open snusnu opened this issue 12 years ago • 10 comments

I'd find quite a few places in my code where I would benefit from being able to do the following:

class Person
  include Concord.new(:name)
end

class Student < Person
  # This would inspect the superclass and
  # if it is concord infected, takes the appropriate
  # params, passes them to super() and sets
  # the remaining ones as ivars just as usual
  include Concord.new(:name, :student_nr)
end

If this sounds interesting, I could provide the respective PR

snusnu avatar Apr 12 '14 09:04 snusnu

We have include anima.add(:some, :attributes) I think we should add include concord.add(:foo, :bar) for symetry. If we reference the superclasses metadata via calling concord and anima singleton methods its more explicit we inherit behavior.

mbj avatar Apr 12 '14 22:04 mbj

@mbj agreed, basically. what about symmetry in case of anima.remove?

snusnu avatar Apr 12 '14 22:04 snusnu

@snusnu Why not? I'm generally a bit defensive adding features to this libs. For anima I cleanly saw the point. For concord I never had the use case.

mbj avatar Apr 12 '14 22:04 mbj

@mbj i'm just wondering what concord.remove(:foo) would do, i.e. how it would call super().

snusnu avatar Apr 12 '14 22:04 snusnu

@snusnu I think you need to reparse your quesition and outline it with code. Are you talking about a super or zsuper call in the generated initializer? Or an implementation detail of the concord inherit feature you are planning?

mbj avatar Apr 13 '14 11:04 mbj

@mbj The way I see it, concord.remove(:foo) doesn't make any sense because the superclass constructor would expect foo and there's no way substitute that automatically. However, concord.add(:foo) makes perfect sense, since it would simply do a super call passing the attributes the superclass expects, and assigning @foo = foo after that call.

class Person
  include Concord.new(:name)
end

class Student < Person
  include concord.add(:student_nr)

  # would generate
  # def initialize(name, student_nr)
  #   super(name)
  #   @student_nr = student_nr
  # end
end

I only mentioned concord.remove(:foo) because you were talking about symmetry with anima. However, with concord it doesn't really make sense if one wants to rely on the superclass #initialize to be called. Granted, for "pure" concord initializers, it might not make a difference if super is called or not, but with the latest addition of being able to call super/zsuper inside a custom #initialize, it seems appropriate to perform the respective super call.

snusnu avatar Apr 13 '14 12:04 snusnu

@snusnu What about NOT calling super in the generated constructor all?

mbj avatar Apr 13 '14 14:04 mbj

@mbj that's the thing, i think it'd be cleaner to call super. At least it's what i would expect. Like I said, for "pure" concord generated #initialize it might not matter, but with the recent addition of allowing to call super from a "overwritten" #initialize, it might surprise users if super wouldn't work anymore ...

Am I missing something?

snusnu avatar Apr 13 '14 16:04 snusnu

@snusnu Lets only add what we need. So lets drop #remove for now. And only add #add. Once we need #remove we can consider it, and than we also see required semantics.

mbj avatar Apr 13 '14 18:04 mbj

@mbj ack

snusnu avatar Apr 13 '14 20:04 snusnu