jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

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

Open ppibburr opened 5 years ago

ppibburr commented 5 years ago

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 commented 5 years ago

Is this something similar to what Vala does?

ppibburr commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

quark_data would be suitable I believe.

ppibburr commented 5 years ago

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 commented 5 years ago
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