Re: [lmi] Resolving TransferString() TODO comment

[Greg Chicares]

On 2016-06-26 22:10, Vadim Zeitlin wrote:
[...]
>       https://github.com/vadz/lmi/pull/43/
> 
> As the commit message hints, I'm not really sure if it's a net gain. There

I agree that it's not a net gain.

> are definitely some advantages to not using a template, as mention in the
> comment in my PR, but the stutter in the calls to this function looks ugly
> and this also allows passing 2 different objects to it which really
> shouldn't be possible.

TransferString() has long been a function template. The disadvantages
cited in your PR are:

"instantiating it many times for different classes": It looks like there
should be five instantiations, one for each of these classes:
  {wxCheckListBox, wxChoice, wxComboBox, wxListBox, wxRadioBox}
and they should all be confined to this single translation unit where
the template is declared in an unnamed namespace.

"better compilation error messages": That's a general argument for
avoiding templates in situations where they offer little benefit.

Against these general disadvantages, the advantage of templates here is
that they make code more succinct. And we'd still have function templates
for TransferBool() and TransferInt() anyway.

>  In fact the only reasonable solution I see would be to add a
> GetContainingWindow() accessor to wxItemContainerImmutable itself as I
> think it could be useful elsewhere and we do have similar methods in other
> similar classes, e.g. wxTextEntry has GetEditableWindow(). Then we could
> take just a wxItemContainerImmutable in TransferString() and everything
> would work nicely and simply.

A similar argument could be made for adding a wxControlWithValue base
class, to provide SetValue() and GetValue(), so that TransferBool() and
TransferInt() wouldn't have to be function templates. But there's no need
to do that: the templates are okay.

When I originally wrote this code, either wxControlWithItems didn't yet
exist, or I was unaware of it. I added that "TODO ??" comment to suggest
investigating whether using wxControlWithItems would make the code better.
Now we've investigated that, and the change I'll apply is this:

>  Of course, another solution is to just drop the "TODO ??" comment and live
> with the current template function