wxruby development - Feb 2009 - Little improvement to Toolbar#add_item

Hi,

In the RichTextCtrl sample, I have the following code :

    toolbar.add_item(undo_bmp, :bitmap2 => undo_disabled_bmp, :id =>
Wx::ID_UNDO,
      :label => "Undo", :short_help => "Undo change")

I find it a bit ugly ...

I''d really like to be able to write as following :

    toolbar.add_item([undo_bmp, undo_disabled_bmp], :id => Wx::ID_UNDO,
      :label => "Undo", :short_help => "Undo change")

If the first argument is an array with two elements :
- the first element corresponds to the normal bitmap
- the second element corresponds to the disabled bitmap

If the first argument is not an array, then it is expected to be the
normal bitmap as usual.

Any objection ?

Chauk-Mean.

Chauk-Mean Proum wrote:
> Hi,
>
> In the RichTextCtrl sample, I have the following code :
>
>     toolbar.add_item(undo_bmp, :bitmap2 => undo_disabled_bmp, :id =>
> Wx::ID_UNDO,
>       :label => "Undo", :short_help => "Undo
change")
>
> I find it a bit ugly ...
>
> I''d really like to be able to write as following :
>
>     toolbar.add_item([undo_bmp, undo_disabled_bmp], :id => Wx::ID_UNDO,
>       :label => "Undo", :short_help => "Undo
change")
>
> If the first argument is an array with two elements :
> - the first element corresponds to the normal bitmap
> - the second element corresponds to the disabled bitmap
>   
Personally, I don''t like it much. It feels a bit too complicated, just 
to save typing :bitmap2 => ...

Maybe I''m just making up conventions as we go along, but as a rule it 
holds true: first (positional) args are essential, later (maybe keyword) 
args modify behaviour. I think the disabled bitmap is the latter?

alex

2009/2/17 Alex Fenton <alex at pressure.to>:
> Chauk-Mean Proum wrote:
>> I''d really like to be able to write as following :
>>
>>    toolbar.add_item([undo_bmp, undo_disabled_bmp], :id =>
Wx::ID_UNDO,
>>      :label => "Undo", :short_help => "Undo
change")
>>
>> If the first argument is an array with two elements :
>> - the first element corresponds to the normal bitmap
>> - the second element corresponds to the disabled bitmap
>>
>
> Personally, I don''t like it much. It feels a bit too complicated,
just to
> save typing :bitmap2 => ...
Well, it seems logical to provide an array of 2 bitmaps if we need
these 2 bitmaps.
I don''t see it as complicated but it''s a matter of taste.
> Maybe I''m just making up conventions as we go along, but as a rule
it holds
> true: first (positional) args are essential, later (maybe keyword) args
> modify behaviour. I think the disabled bitmap is the latter?
>
As an alternative to the array proposal, would you prefer that the
bitmap2 argument is the second positional argument ?
Thus, I could rewrite my code as following :

toolbar.add_item(undo_bmp, undo_disabled_bmp, :id => Wx::ID_UNDO,
:label => "Undo", :short_help => "Undo change")


Chauk-Mean.

Chauk-Mean Proum wrote:
> Well, it seems logical to provide an array of 2 bitmaps if we need
> these 2 bitmaps.
The disabled bitmap isn''t *needed*, in general. It''s an
optional
feature, maybe useful in some cases, but irrelevant in others: it''s 
omitted in wxWidgets "shorter and more commonly used" signature for
AddTool.
> As an alternative to the array proposal, would you prefer that the
> bitmap2 argument is the second positional argument ?
> Thus, I could rewrite my code as following :
>
> toolbar.add_item(undo_bmp, undo_disabled_bmp, :id => Wx::ID_UNDO,
> :label => "Undo", :short_help => "Undo change")
I don''t wish to be obdurate, but not really; it just doesn''t
seem to
solve any core, general problem. Why give it special treatment over 
other optional Tool features (eg "short help string"), at the cost of 
writing and maintaining more code and documentation?

If the treatment of an optional feature is important to a particular app 
or programmer, it''s easily dealt with at the level of that application 
or the user''s personal lib.

:)
alex

I''ve been following this, and I hate to say it, but I''m with
Alex on this
one Chauk.  Disable Bitmap is a optional argument, that isn''t required
for
the toolbar item to work.  The promotion of optional arguments to primary
arguments should be done on a per coder / per project basis.  And what I
mean by that, is that it is very easy to subclass the toolbar class, and
change the way add_item works.  Heck, you can even do the following:

class << toolbar
  alias :old_add_item :add_item
  def add_item(your,own,signature,here)
    ... remux the signature into a wxRuby signature ...
    old_add_item(*remux_signature_to_call)
  end
end

This part, is really needed to be up to the individual programmer, not
forced upon them by changing an optional parameter into a required one.

Mario

On Wed, Feb 18, 2009 at 8:04 AM, Alex Fenton <alex at pressure.to> wrote:
> Chauk-Mean Proum wrote:
>
>> Well, it seems logical to provide an array of 2 bitmaps if we need
>> these 2 bitmaps.
>>
>
> The disabled bitmap isn''t *needed*, in general. It''s an
optional feature,
> maybe useful in some cases, but irrelevant in others: it''s omitted
in
> wxWidgets "shorter and more commonly used" signature for AddTool.
>
>  As an alternative to the array proposal, would you prefer that the
>> bitmap2 argument is the second positional argument ?
>> Thus, I could rewrite my code as following :
>>
>> toolbar.add_item(undo_bmp, undo_disabled_bmp, :id => Wx::ID_UNDO,
>> :label => "Undo", :short_help => "Undo
change")
>>
>
> I don''t wish to be obdurate, but not really; it just
doesn''t seem to solve
> any core, general problem. Why give it special treatment over other
optional
> Tool features (eg "short help string"), at the cost of writing
and
> maintaining more code and documentation?
>
> If the treatment of an optional feature is important to a particular app or
> programmer, it''s easily dealt with at the level of that
application or the
> user''s personal lib.
>
> :)
> alex
>
> _______________________________________________
> wxruby-development mailing list
> wxruby-development at rubyforge.org
> http://rubyforge.org/mailman/listinfo/wxruby-development
>


-- 
Mario Steele
http://www.trilake.net
http://www.ruby-im.net
http://rubyforge.org/projects/wxruby/
http://rubyforge.org/projects/wxride/
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/wxruby-development/attachments/20090218/8817d517/attachment-0001.html>

2009/2/18 Alex Fenton <alex at pressure.to>:
> Chauk-Mean Proum wrote:
>>
>> Well, it seems logical to provide an array of 2 bitmaps if we need
>> these 2 bitmaps.
>
> The disabled bitmap isn''t *needed*, in general. It''s an
optional feature,
> maybe useful in some cases, but irrelevant in others:
I fully agree. That''s the reason why :
- if you need only a bitmap, you supply a bitmap
- if you need two bitmaps, you supply the 2 bitmaps in an array

>> As an alternative to the array proposal, would you prefer that the
>> bitmap2 argument is the second positional argument ?
>> Thus, I could rewrite my code as following :
>>
>> toolbar.add_item(undo_bmp, undo_disabled_bmp, :id => Wx::ID_UNDO,
>> :label => "Undo", :short_help => "Undo
change")
>
> I don''t wish to be obdurate, but not really; it just
doesn''t seem to solve
> any core, general problem.
> Why give it special treatment over other optional
> Tool features (eg "short help string"),
There is no core or general problem.
As I indicated earlier, it''s more logical to group things that are
logically related (even if one of the thing is an optional element).
A disabled bitmap is logically related with the normal bitmap.
This is not the case with other features like "short help string".
> at the cost of writing and maintaining more code and documentation?
Yes, I have to modify the code and the doc but I don''t understand the
maintenance costs here.
It is just a little modification of a convenience/additional method I
have proposed several months ago.


Chauk-Mean.

Hi Mario,

2009/2/18 Mario Steele <mario at ruby-im.net>:
> I''ve been following this, and I hate to say it, but I''m
with Alex on this
> one Chauk.  Disable Bitmap is a optional argument, that isn''t
required for
> the toolbar item to work.  The promotion of optional arguments to primary
> arguments should be done on a per coder / per project basis.
 ...
> This part, is really needed to be up to the individual programmer, not
> forced upon them by changing an optional parameter into a required one.

Oups, there is a clear misunderstanding here ...

I''ve never proposed to make the disabled bitmap a required parameter.

If disabled bitmap is needed/used, I just proposed to "place it near"
the normal bitmap :
a) First proposal :
  - in an array of bitmaps supplied as the first mandatory paramater.
  - if disabled bitmap is *not* needed/used, the normal bitmap is
supplied directly as the first mandatory parameter.
or
b) Second proposal :
  - as the second positional parameter but it still remains an
optional parameter

I hope that this will clarify my intention/proposals.

Chauk-Mean.

Chauk-Mean Proum wrote:
> I fully agree. That''s the reason why :
> - if you need only a bitmap, you supply a bitmap
> - if you need two bitmaps, you supply the 2 bitmaps in an array
>   
AFAIK, we don''t use "Object or Array-of-Object" to signal
optional
arguments anywhere else in the library. We do make use of keyword 
arguments for just this purpose, and that''s what the current way does.
> There is no core or general problem.
>   
In that case, why do it? Let''s leave well alone. We must try to avoid 
special-casing and tinkering to fix a non-problem (or rather, a minor 
aesthetic problem in your eyes only).
> As I indicated earlier, it''s more logical to group things that are
> logically related (even if one of the thing is an optional element).
You think it is, I don''t; I think it''s more logical to have
all optional
arguments as keywords in the API.
>> Yes, I have to modify the code and the doc but I don''t
understand the
>> maintenance costs here.
>>     
The more code there is, the more to break, as development proceeds.
> It is just a little modification of a convenience/additional method I
> have proposed several months ago.
Which I liked and added, because it met the kind of criteria for a good 
syntax change/addition:

- make similar methods work similarly in the wxRuby API
- make methods more Ruby-ish (eg blocks for scoped effects)
- make hard things easier
- make code more self-documenting

I don''t think this proposal does any of those things. I''m fine
if you
want to use this idiom in a sample. But I think it''s of no benefit in 
general to the library at the moment; we can review it later as part of 
more thoroughgoing changes for a wxRuby 3.0...

alex

Alex,

It seems that there is still some misunderstanding.

2009/2/18 Alex Fenton <alex at pressure.to>:
> Chauk-Mean Proum wrote:
>>
>> I fully agree. That''s the reason why :
>> - if you need only a bitmap, you supply a bitmap
>> - if you need two bitmaps, you supply the 2 bitmaps in an array
>
> AFAIK, we don''t use "Object or Array-of-Object" to
signal optional arguments
> anywhere else in the library. We do make use of keyword arguments for just
> this purpose, and that''s what the current way does.
I agree. And as you don''t like the array proposal, I made a second
proposal.
>> There is no core or general problem.
>
> In that case, why do it? Let''s leave well alone. We must try to
avoid
> special-casing and tinkering to fix a non-problem (or rather, a minor
> aesthetic problem in your eyes only).
>>
>> As I indicated earlier, it''s more logical to group things that
are
>> logically related (even if one of the thing is an optional element).
>
> You think it is, I don''t; I think it''s more logical to
have all optional
> arguments as keywords in the API.
But it is still the case in my second proposal :

ToolBarTool ToolBar#add_item(Bitmap bitmap, Bitmap bitmap2 NULL_BITMAP, Integer
position = -1, Integer id = -1, String label ='''',
ItemKind kind = ITEM_NORMAL, String short_help = '''', String
long_help
= '''', Object client_data = nil)

This proposal is just to define bitmap2 as the second argument to
allow writing :

toolbar.add_item( undo_bmp, undo_disabled_bmp, :id => Wx::ID_UNDO,
    :label => "Undo", :short_help => "Undo change")

but you can also write it as :

toolbar.add_item(undo_bmp, :bitmap2 => undo_disabled_bmp,
    :id =>Wx::ID_UNDO, :label => "Undo", :short_help =>
"Undo change")

bitmap2 is still an optional keyword argument. It is just defined as
the second positional argument.

I don''t think that my second proposal introduces some inconsistency
here.

Anyway, I give up ...

Chauk-Mean.