Hi Attached is a patch that adds aliases to the API for methods that look like attribute accessors. So a_frame.set_title(''The title'') # currently a_frame.title = ''The title'' # now an alternative textctrl.get_value # currently textctrl.value # now an alternative Also, C++ methods named ''IsXXX'' are now exposed in ruby with the alternative name ''xxx?'', eg tree_item_id.is_ok # currently tree_item_id.ok? # now an alternative a_checkbox.is_checked a_checkbox.checked? Note these are simply aliases - existing code can remain unchanged. I think this would do a lot to make the API feel more ruby-like (or pythonesque - WxPython follows this style) - but is pretty cheap and straightforward. There is no speed hit, and the resulting binary is only marginally larger (<1% on my build). I will follow this patch with updates to the doc translator if needed. What do people think? alex _______________________________________________ wxruby-users mailing list wxruby-users@rubyforge.org http://rubyforge.org/mailman/listinfo/wxruby-users
Alex, That looks good to me, less typing required. :) I have not tried the patch but the code looks like it will work and I can''t think of any corner cases off the top of my head. Sean On 9/11/06, Alex Fenton <alex at pressure.to> wrote:> Hi > > Attached is a patch that adds aliases to the API for methods that look > like attribute accessors. So > > a_frame.set_title(''The title'') # currently > a_frame.title = ''The title'' # now an alternative > > textctrl.get_value # currently > textctrl.value # now an alternative > > Also, C++ methods named ''IsXXX'' are now exposed in ruby with the > alternative name ''xxx?'', eg > > tree_item_id.is_ok # currently > tree_item_id.ok? # now an alternative > > a_checkbox.is_checked > a_checkbox.checked? > > Note these are simply aliases - existing code can remain unchanged. I > think this would do a lot to make the API feel more ruby-like (or > pythonesque - WxPython follows this style) - but is pretty cheap and > straightforward. There is no speed hit, and the resulting binary is only > marginally larger (<1% on my build). > > I will follow this patch with updates to the doc translator if needed. > > What do people think? > alex > > > > > > Index: wxruby2/swig/renamer.rb > ==================================================================> RCS file: /var/cvs/wxruby/wxruby2/swig/renamer.rb,v > retrieving revision 1.4 > diff -b -u -r1.4 renamer.rb > --- wxruby2/swig/renamer.rb 3 Sep 2005 21:03:47 -0000 1.4 > +++ wxruby2/swig/renamer.rb 11 Sep 2006 22:51:32 -0000 > @@ -45,6 +45,18 @@ > new_method_name = ''"'' + un_camelcase(method_name) + ''"'' > line[quoted_method_name] = new_method_name > #puts(line) > + if new_method_name =~ /^"([gs]et|is)_(.*)"$/ > + line2 = line.dup > + case $1 > + when ''get'' > + line2[new_method_name] = "\"#{$2}\"" > + when ''set'' > + line2[new_method_name] = "\"#{$2}=\"" > + when ''is'' > + line2[new_method_name] = "\"#{$2}?\"" > + end > + line << "\n" << line2 > + end > end > return line > end > > _______________________________________________ > wxruby-users mailing list > wxruby-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/wxruby-users > >
Alex Fenton wrote:> Attached is a patch that adds aliases to the API for methods that look > like attribute accessors. > > Note these are simply aliases - existing code can remain unchanged.Generally this looks reasonable. My only concern is that we are actually creating a second, independent method at the Ruby level, which could cause problems. I haven''t thought it through, but I wonder if there is a corner case where someone might extend a class (say MyFrame < Frame) and override one of these methods. Then they extend that class (MyFooFrame). At that level, I suspect we would actually have two completely separate methods, so one would be overridden and the other would not, possibly leading to surprises. At that point, is_ok and ok? would behave differently. Would it be safer to actually create true Ruby aliases for each of these methods? If so, this could be done in wxsugar (or wx.rb), or as a post-swig processor adding a define_alias call after each define_method call. Or maybe doing so would not actually help with the potential problem I mentioned. Perhaps we just need to tell people to be consistent using one form or the other, because mixing them is inherently dangerous. Kevin
Alex Fenton wrote:> Attached is a patch that adds aliases to the API for methods that look > like attribute accessors.Sorry to fork my own thread, but I had another thought that seems to fit better here: Should this actually go in wxsugar, at least for now? When possible, I would like to have experimental code be in ruby (not C++ or SWIG), and not be in the core product. As it gets refined and becomes widely accepted, it could be folded in to wxruby proper. Just a thought. I''m still open to both approaches. Kevin
Hi Kevin> My only concern is that we are actually creating a second, independent method at the Ruby level, which could cause problems. >A good point - but actually, this is exactly what the alias and alias_method do in Ruby - create a copy, not a reference. So if you subclass and redefine a method which has an alias in the superclass, the alias in the subclass will still have the original (superclass) definition.> Would it be safer to actually create true Ruby aliases for each of these > methods? If so, this could be done in wxsugar (or wx.rb), or as a > post-swig processor adding a define_alias call after each define_method > call. >Yup, WxSugar accessors.rb does the former. But b/c of the above, the main difference is the hit to startup speed of doing it in ruby, b/c it has to reflect every ruby class and method on startup.> Perhaps we just need to tell people to be consistent using > one form or the other, because mixing them is inherently dangerous. >Yep, I agree, though in practice I''ve been mixing them for two years without problem. The main irritation is remembering that ''foo ='' is interpreted as assignment to a local variable, so you must say ''self.foo='' - but this is a general Ruby wrinkle. Or in the much longer term, maybe even deprecate and remove the C++ish names ;) - but I''m definitely not suggesting that now. cheers alex
Alex Fenton wrote:> A good point - but actually, this is exactly what the alias and > alias_method do in Ruby - create a copy, not a reference.Oh. Right.> Yup, WxSugar accessors.rb does the former. But b/c of the above, the > main difference is the hit to startup speed of doing it in ruby, b/c it > has to reflect every ruby class and method on startup.Can you quantify that? We''re already loading a large library. Are we talking 1.1 seconds instead of 1? Or 10 seconds instead of 5? And while we are in an alpha state anyway, is speed one of our highest priorities?> Yep, I agree, though in practice I''ve been mixing them for two years > without problem. The main irritation is remembering that ''foo ='' is > interpreted as assignment to a local variable, so you must say > ''self.foo='' - but this is a general Ruby wrinkle.Ugh. Does ''@foo ='' work in that case?> Or in the much longer term, maybe even deprecate and remove the C++ish > names ;) - but I''m definitely not suggesting that now.Yes, in the long run I could imagine wxruby only supporting the sugary versions. Maybe at that point have a wxpepper module that simulates the original C++ API for folks who hate sugar :-) Kevin
Kevin Smith wrote:> Can you quantify that?25-30% above wx2 (which is about 2.25s here) accessors.rb is about 80% of the start-up time of WxSugar.>> Yep, I agree, though in practice I''ve been mixing them for two years >> without problem. The main irritation is remembering that ''foo ='' is >> interpreted as assignment to a local variable, so you must say >> ''self.foo='' - but this is a general Ruby wrinkle. >> > > Ugh. Does ''@foo ='' work in that case? >Nope, because it''s still a method call not a variable assignment. Which is ruby''s problem - when there isn''t a scope sigil (like @) or an explicit callee on the LHS, ruby''s parser has no definitive way of telling a local var assignment from a method call on self. So it has to guess one.> Maybe at that point have a wxpepper module that simulates the > original C++ API for folks who hate sugar :-) >LOL! alex
Kevin Smith wrote:> Sorry to fork my own thread, but I had another thought that seems to fit > better here: Should this actually go in wxsugar, at least for now? >I''d like to commit this b/c I think it will improve the API''s acceptability and usefulness (to some) and is worth having in the core, relative to the risks. Let''s see what feedback we get from a more public release with both styles available. Maybe run a survey on rubyforge? It''s not a hard patch to back out of at this stage.> When possible, I would like to have experimental codeI still find wxruby2 a bit ''experimental'' generally...> be in ruby > (not > C++ or SWIG),Yes for sure. People being able to contribute to wxruby in ruby seems to me impt to attracting developers.> and not be in the core product. As it gets refined and > becomes widely accepted, it could be folded in to wxruby proper. >Yep. If we''re going to use wx_sugar for this purpose, can we run it through wxruby rubyforge project as an SVN module so others have access to it? I don''t want to open weft-qda CVS. alex
Alex Fenton wrote:> I''d like to commit this b/c I think it will improve the API''s > acceptability and usefulness (to some) and is worth having in the core, > relative to the risks. Let''s see what feedback we get from a more public > release with both styles available. Maybe run a survey on rubyforge?The thing is...once it''s in a wxruby release, there would be a lot of resistance to removing it...even if it causes problems. I view its inclusion as a strong commitment, and I''m not sure I''m ready to take that step.> If we''re going to use wx_sugar for this purpose, can we run it through > wxruby rubyforge project as an SVN module so others have access to it? I > don''t want to open weft-qda CVS.We could host wxsugar within the wxruby rubyforge project. Or wxsugar could be its own rubyforge project. I don''t have strong feelings for or against either approach. Kevin
Alex Fenton wrote:>> Can you quantify that? > 25-30% above wx2 (which is about 2.25s here) > accessors.rb is about 80% of the start-up time of WxSugar.To me, half a second extra to launch a GUI app is not worth worrying about at this alpha stage of our project. Based on that, I would vote to leave it in ruby for now. I wonder if there''s a way to hook the different classes so the accessors are only created when a class is first referenced. Since we have hundreds of classes, but small apps would typically only use a handful of them, it could be a big win.>>> Yep, I agree, though in practice I''ve been mixing them for two years >>> without problem. The main irritation is remembering that ''foo ='' is >>> interpreted as assignment to a local variable, so you must say >>> ''self.foo='' - but this is a general Ruby wrinkle. >>> >> Ugh. Does ''@foo ='' work in that case? >> > Nope, because it''s still a method call not a variable assignment. Which > is ruby''s problem - when there isn''t a scope sigil (like @) or an > explicit callee on the LHS, ruby''s parser has no definitive way of > telling a local var assignment from a method call on self. So it has to > guess one.Blech! To me that may be reason enough not to support write accessors. An API that strongly invites people to make a mistake that silently does the wrong thing is very unappealing to me. Very very unappealing. Kevin
> I wonder if there''s a way to hook the different classes so the accessors > are only created when a class is first referenced. Since we have > hundreds of classes, but small apps would typically only use a handful > of them, it could be a big win.Probably could do this with Object#method_missing check if matches regex and then use Module#define_method. If I had the time I would try doing it, maybe someone else can run with this idea. Sean
Sean Long wrote:> Probably could do this with Object#method_missing check if matches > regex and then use Module#define_method.Thanks for the idea - method_missing works well here and doesn''t have the startup hit. Anyway, now implemented like this in WxSugar. def method_missing(sym, *args) case sym.to_s when /^(.*)\=$/ meth = "set_#{$1}" when /^(.*)\?$/ meth = "is_#{$1}" else meth = "get_#{sym}" end if respond_to?(meth) send(meth, *args) else e = NoMethodError.new("undefined method ''#{sym}'' for #{self.inspect}") e.set_backtrace(caller) Kernel.raise e end end
Kevin Smith wrote:> The thing is...once it''s in a wxruby release, there would be a lot of > resistance to removing it...even if it causes problems.Sure. But as with any other code, we try an informed guess as to whether it will case problems (and we haven''t come up with any yet) and then try it to see if it really does. Anyway, since method_missing reduces the startup cost of doing it in Ruby, I''m willing to leave this out for now til we solicit a wider range of feedback via a public release etc. I think there are arguments against having this in, e.g. "I prefer the set_ get_ is_ style" (which is fine) or "having two naming schemes is confusing" (in which case we need to pick one and stick with it). But this stage of the project would seem to be the time to try out API-changing changes, when we''re not committed to maintaining lots of other people''s code. alex
Alex Fenton wrote:> I think there are arguments against having this in, e.g. "I prefer the > set_ get_ is_ style" (which is fine) or "having two naming schemes is > confusing" (in which case we need to pick one and stick with it).Neither of these are of concern to me. Well, the first one is slightly, because of all the existing non-Ruby wx documentation, but we have already mangled GetXxx into get_xxx. The real issues for me are whether xxx= is going to cause excessive frustration (because it needs to be self.xxx=), and whether people will run into technical problems from mixing the two styles (perhaps inadvertently by using two third-party wxruby extensions).> But > this stage of the project would seem to be the time to try out > API-changing changes, when we''re not committed to maintaining lots of > other people''s code.Absolutely, which is why having it in an optional, unofficial package is perfect. If the implementation proves to be solid, and the community likes it, then it would make sense to pull it into wxruby core. Thanks, Kevin
Nice! That is what I had in mind. I can''t wait until I get more time to actually do something on the project besides read/writing emails. :) Sean On 9/15/06, Alex Fenton <alex at pressure.to> wrote:> Sean Long wrote: > > Probably could do this with Object#method_missing check if matches > > regex and then use Module#define_method. > Thanks for the idea - method_missing works well here and doesn''t have > the startup hit. > > Anyway, now implemented like this in WxSugar. > > def method_missing(sym, *args) > case sym.to_s > when /^(.*)\=$/ > meth = "set_#{$1}" > when /^(.*)\?$/ > meth = "is_#{$1}" > else > meth = "get_#{sym}" > end > if respond_to?(meth) > send(meth, *args) > else > e = NoMethodError.new("undefined method ''#{sym}'' for #{self.inspect}") > e.set_backtrace(caller) > Kernel.raise e > end > end > > _______________________________________________ > wxruby-users mailing list > wxruby-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/wxruby-users >
Alex Fenton hat geschrieben:> Anyway, now implemented like this in WxSugar. > > def method_missing(sym, *args) > case sym.to_s > when /^(.*)\=$/ > meth = "set_#{$1}" > when /^(.*)\?$/ > meth = "is_#{$1}" > else > meth = "get_#{sym}" > end > if respond_to?(meth) > send(meth, *args) > else> e = NoMethodError.new("undefined method ''#{sym}'' for #{self.inspect}") > e.set_backtrace(caller) > Kernel.raise eJust a little note about the code above. If all you want is the standard behaviour of method_missing, you can do it this way: super> end > end