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

Generate proper initializers for concord infected inheritance trees #7

Open
snusnu opened this issue Apr 12, 2014 · 10 comments
Open

Generate proper initializers for concord infected inheritance trees #7

snusnu opened this issue Apr 12, 2014 · 10 comments

Comments

@snusnu
Copy link
Collaborator

snusnu commented Apr 12, 2014

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

@mbj
Copy link
Owner

mbj commented Apr 12, 2014

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.

@snusnu
Copy link
Collaborator Author

snusnu commented Apr 12, 2014

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

@mbj
Copy link
Owner

mbj commented Apr 12, 2014

@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.

@snusnu
Copy link
Collaborator Author

snusnu commented Apr 12, 2014

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

@mbj
Copy link
Owner

mbj commented Apr 13, 2014

@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?

@snusnu
Copy link
Collaborator Author

snusnu commented Apr 13, 2014

@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.

@mbj
Copy link
Owner

mbj commented Apr 13, 2014

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

@snusnu
Copy link
Collaborator Author

snusnu commented Apr 13, 2014

@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?

@mbj
Copy link
Owner

mbj commented Apr 13, 2014

@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.

@snusnu
Copy link
Collaborator Author

snusnu commented Apr 13, 2014

@mbj ack

@mbj mbj mentioned this issue Oct 20, 2014
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