GitHub user NightOwl888 opened a pull request:
https://github.com/apache/lucenenet/pull/207
API: Added ReferenceManager<G>.AcquireContext()
Opening this as a pull request so the team can discuss this approach vs the
previous `ExecuteSearch` method (that was inadvertently removed from the API).
## AcquireContext Example
```c#
var searcherManager = new SearcherManager(indexWriter, true, null);
using (var context = searcherManager.AcquireContext())
{
IndexSearcher searcher = context.Reference;
var topDocs = searcher.Search(query, 10);
// use results...
}
```
### Pros
1. Eliminates the try-finally block.
2. Doesn't swallow exceptions by default.
3. Implicitly cleans up.
4. Since the extension method is on `ReferenceManager<G>`, all of its
subclasses automatically gain this functionality.
### Cons
1. If the user forgets the using block, cleanup is not automatic.
## ExecuteSearch Example
```c#
var searcherManager = new SearcherManager(indexWriter, true, null);
searcherManager.ExecuteSearch(searcher =>
{
var topDocs = searcher.Search(query, 10);
// use results...
}, exception => { Console.WriteLine(exception.ToString()); });
```
### Pros
1. Eliminates the try-finally block.
2. Implicitly cleans up.
3. `ExecuteSearch` name makes it easier to find on the API.
4. The user doesn't need to remember any special syntax to clean up.
### Cons
1. Swallows exceptions by default. To make the exception bubble, you have
to re-throw it.
2. API only applies to SearcherManager, so similar code needs to be
repeated for other `ReferenceManager<G>` subclasses.
### Discussion
Although the `ExecuteSearch` method is well named and thus easier to find
on the API, it has the disadvantage of swallowing exceptions by default.
Exceptions do not automatically bubble - you have to re-throw the exception (in
which case performance suffers and you lose the stack trace). Its main
advantage is that there is no way for the user to accidentally forget to clean
up.
The pattern seems to lend itself better to a using block. If all code
examples are shown with a using block the user is not likely to forget it.
However, admittedly, `AcquireContext` and `context.Reference` are probably not
the most intuitive names (I was torn between `Context` or `Holder` for the
name, but there might be a better choice than either of those).
`context.Reference` was chosen because `ReferenceContext` is the name of the
generic class that holds the reference and it applies to all subclasses of
`ReferenceManager<G>`. Of course, the original API was named `Acquire()`, so it
seems logical to use `AcquireContext()` or `AcquireHolder()` or something along
those lines. Ideas welcome.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/NightOwl888/lucenenet api-reference-context
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucenenet/pull/207.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #207
----
commit dc9bbc056f678ee2cdd63b0a877c1ff75c37f14a
Author: Shad Storhaug <[email protected]>
Date: 2017-05-11T06:44:46Z
API: Added ReferenceManager<G>.AcquireContext(), which is similar to
ReferenceManager<G>.Acquire() but can be used in a using block to implicitly
dereference instead of having to do it explicitly in a finally block.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
