Xapian devel - Apr 2007 - ExpandDecider and MatchDecider operator() return type

Currently ExpandDecider::operator() and MatchDecider::operator() return
`int' for no very good reason that I can see.  It would be more natural to
return `bool', since these classes are making a "yes/no" decision
about
whether to include a term in an ESet or a document in an MSet.

The problem is that this can't be done without breaking existing user
code which defines subclasses of MatchDecider or ExpandDecider (or at
least I can't see a way since C++ overloading is based on the argument
types only, not the return type).

So previously I've just ignored this, but it's now giving me grief while
trying to create SWIG based Java bindings which are as compatible as
possible with the existing hand-written JNI Java bindings.

I propose this should change in 1.0.  We'll have to skip deprecating the
old way, but updating is easy (just change 'int' to 'bool') and
any
existing code which needs changing will give a pretty clear error, e.g.
GCC 4.1 reports:

    error: conflicting return type specified for 'virtual int
	Xapian::MyExpandDecider::operator()(const std::string&) const'
    error:   overriding 'virtual bool
	Xapian::ExpandDecider::operator()(const std::string&) const'

Thoughts?

Cheers,
    Olly

Olly Betts wrote:
> Currently ExpandDecider::operator() and MatchDecider::operator() return
> `int' for no very good reason that I can see.  It would be more natural
to
> return `bool', since these classes are making a "yes/no"
decision about
> whether to include a term in an ESet or a document in an MSet.
> 
> The problem is that this can't be done without breaking existing user
> code which defines subclasses of MatchDecider or ExpandDecider (or at
> least I can't see a way since C++ overloading is based on the argument
> types only, not the return type).
> 
> So previously I've just ignored this, but it's now giving me grief
while
> trying to create SWIG based Java bindings which are as compatible as
> possible with the existing hand-written JNI Java bindings.
> 
> I propose this should change in 1.0.  We'll have to skip deprecating
the
> old way, but updating is easy (just change 'int' to 'bool')
and any
> existing code which needs changing will give a pretty clear error, e.g.
> GCC 4.1 reports:
I think this is reasonable - it should change at some point, and 1.0 is 
the only sensible time in the near future.

I suspect a lot of people are going to have to make at least minor 
changes to compile with 1.0 anyway, since several of the pieces of code 
I've looked at recently used some of the deprecated functions.

If we did want to follow the deprecation policy, the only way to do it 
that I can think of would be to introduce a whole new hierarchy of 
MatchDecider classes, called, say, "MatchDecider2", which have a 
different method signature, and overload the get_mset() method to take 
one of these instead.  Which seems rather cumbersome, and leaves us with 
a silly name for the class even after 1.1  (I suppose at that point we 
could rename MatchDecider2, and deprecate the old one, so it wouldn't be 
until release 1.2 that we have the state we're aiming for.)

So, just go for it, I think.

-- 
Richard