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

avoid having to redefine 'new' constructors when overloading initialize #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ppibburr
Copy link
Contributor

@ppibburr ppibburr commented Nov 9, 2018

an after initialize hook, not applicable to cast
the macro makes it look less artificial :D (crystal should have this built in :/)

class Mine < Some::GObject
  construct {
    # called after initialize
  }

  ## or
  # def construct
  #   # ...
  # end
end

@megatux
Copy link

megatux commented Nov 9, 2018

Is this something similar to what Vala does?

@ppibburr
Copy link
Contributor Author

ppibburr commented Nov 9, 2018

inspired by Vala and in regards to Vala's construct block, yes, when initialization code should always be run, regardless of the constructor invoked, use the construct block.

@jhass
Copy link
Owner

jhass commented Nov 10, 2018

I'm not sure about the name, Crystal inherited the initialize/finalize terminology from Ruby, so constructor feels alien. I'd either side-step the issue with using setup or configure or so, or be explicit by using after_initialize or on_initialize

I'm not sure I get the point of the macro, just doing def setup seems as clean? We we have a reason to stick to the macro the calls should use do/end blocks btw.

Finally I wonder if you thought about just calling it from the generated def initialize, might be simpler? Or am I missing something that prevents that?

@ppibburr
Copy link
Contributor Author

The naming could be any name. Just rolled with what I was familiar with.

As for calling from initialize. cast also will call initialize so I placed it in the constructor methods.

@jhass
Copy link
Owner

jhass commented Nov 10, 2018

I'm probably not seeing something, so bear with me please, but why don't we want the hook to run from classes instantiated via cast?

@ppibburr
Copy link
Contributor Author

wouldn't the constructor code be being called too much?

say i have a container with children of MyType, the children have thier states changed by the user, I then iter the children of the container, cast would run the constructor code again?

@jhass
Copy link
Owner

jhass commented Nov 10, 2018

Maybe cast is named wrong, it's not meant to replace crystal's as, it's for converting a more generic instance returned by a binding to a more specific binding type.

@ppibburr
Copy link
Contributor Author

Heres an example that demonstrates why the hook in initialize woud not best.

require "../src/gtk/autorun"

class MyWidget < Gtk::Button
  def initialize(ptr)
    super

    setup
  end

  def setup
    self.label = "Default Label"
  end

  def self.new
    super
  end
end

w = Gtk::Window.new
w.add v=Gtk::VBox.new(false,0)
3.times do |i|
  v.pack_start(mw=MyWidget.new, false, false, 1)
  mw.label = "button#{i}"
  mw.on_clicked do
    v.foreach(->(c : LibGtk::Widget*, d : Void*) {p MyWidget.new(c.as(LibGtk::Button*)); nil}, nil)
  end
end
w.show_all

By default MyWidget will have a default label. In the code, their labels are changed afterwards. however, once i iterate over the children of the Gtk::VBox using Gtk::Container#foreach to get a MyWidget cast from the child, initialize is called again, and all of the childrens labels revert to default

@jhass
Copy link
Owner

jhass commented Nov 13, 2018

Mh, otoh. wouldn't we want it to run if you fetch it from say Gtk::Builder and then cast to your custom class, at least for the first time? There gotta be a design that's nice & obvious for either case. Maybe there's a sort of tag we can set on an gobject to memoize whether the setup was run?

@ppibburr
Copy link
Contributor Author

quark_data would be suitable I believe.

@ppibburr
Copy link
Contributor Author

This now allows constructor and cast calls to fire the instantiated code at the proper time.
It also allows to yield or return the exact subclass instance if one chooses to via WrappedType#keep_wrapper.

The hook in initialize remains troublesome, However in WrappedType.cast seems best

currently the chain from a constructor call is...
MyType.new -> SuperType.new -> cast CommonType.new(ptr)

CommonType.new would be the first initailize so we would check if object has been wrapped, it hasn't so we set it as wrapped. Now when SuperType#initialize is called the objects already wrapped, so we would NOT call the instantiate code

@ppibburr
Copy link
Contributor Author

class MyType < SuperType
  def instantiate
     ## do stuff
     # ...
     
     ## used to return this exact wrapper
     keep_wrapper
  end
end

container.add mt=MyType.new
container.foreach do |c| 
   _mt = c.as(MyType) 
end

mt.on_some_event do |mt_as_base|
   _mt = mt_as_base.as(MyType)
  _mt == mt
end

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.

3 participants