Opened 16 years ago
Closed 16 years ago
#352 closed defect (fixed)
Weight::init() vs PostingSource::reset()
Reported by: | Olly Betts | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | 1.1.1 |
Component: | Library API | Version: | SVN trunk |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Operating System: | All |
Description
These two methods are sort of equivalent, so perhaps for consistency they should have the same name. Both names are new - neither has yet been in a release, so we should decide if we want to resolve this before 1.1.0 (milestone set appropriately).
Having pondered this for a while, it seems to me that Weight::reset()
is a less good choice than Weight::init()
as we are initialising the object for use, not resetting it. We don't reuse Weight objects - they're cloned for each weighted term in the query. While it's conceivable we could add a cache of Weight objects to Enquire which gets reused if get_mset()
is called a second time, at which point reset()
would make sense, I don't think the extra complexity likely to be justified - one object creation per weighted term is a minimal overhead compared to everything else matching involves.
The doxygen comments for many methods of PostingSource stress that:
Xapian will always call reset() on a PostingSource before calling this for the first time.
This aspect is better suggested by the name init()
, but reset()
suggests better that it can be used more than once (IIRC, originally you didn't need to call reset()
the first time before the Database parameter was added to it, but that's no longer true).
Overall, init()
seems if anything slightly better here too, as well as the consistency benefit.
The flip side is that this means changing a lot of references, and Richard mentioned he had more in some bespoke code.
But it seems to me that it should just be s/\<reset(/init(/
though, followed by a check that reset is never referenced without parentheses (perhaps in documentation) - is there some complication?
Change History (4)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
comment:3 by , 16 years ago
Milestone: | 1.1.0 → 1.1.1 |
---|
The documentation change needn't hold up 1.1.0.
I just mentioned that changing reset to init would involve chasing down a few references as a "tie-breaker" for the decision. On reflection, though, I think that init is indeed a clearer name.
For posting sources, if they don't implement clone(), re-using a posting source will result in init() being called twice. This should be fine, but we should mention in the documentation for the PostingSource::init() method that this is a possibility, and that implementations should cope with it.