Github user NightOwl888 commented on the issue:
https://github.com/apache/lucenenet/pull/207
Looks nice. Although, I tend to favor the "all methods are verbs" and "all
properties are nouns or adjectives" methodology. For example, I would expect a
method named `Buffer()` to buffer something, not return a buffer. In this case
that argument is a bit weaker, but it still feels like `GetContext()` would be
more clear than `Context()`.
I primarily named it `AcquireContext()` because it is essentially the same
functionality as the existing `Acquire()` method (from Java), so the similar
names seem natural. But `GetContext()` is also clear (and more concise).
Hmm, that wasn't what I meant by "other subclasses". Actually, I made the
constructor of `ReferenceContext` internal so it is effectively sealed to the
outside world. What I was referring to is the fact that SearcherManger is just
one of a few (with many more possible) subclasses of `ReferenceManager<G>`.
And since `ReferenceManager<G>` is generic, the `Reference` property is
also generic and the cast is not necessary - its type is specific to the
subclass of `ReferenceManager<G>`.
However, we can't very well name the property `Searcher` (so we have
`context.Searcher`) because it wouldn't be generic enough for the other
subclasses of `ReferenceManager<G>`. Well, we could by making extension methods
and a custom "holder" type for each subclass of `ReferenceManager<G>`. But the
disadvantage there is repetitive similar code. Maybe it would be worth it for
such an integral part of the API, though.
The purpose of the using block here isn't to dispose anything really - it
is just to help ensure the variable isn't utilized after its reference is no
longer tracked. The context is just a "holder" for the reference to implement
`Dispose()` on, which tells the parent `ReferenceManager<G>` class that we are
done using the reference. You can see an example of how it looks with the Java
functions
[here](https://github.com/synhershko/LuceneNetDemo/blob/master/LuceneNetDemo/GitHubIndex.cs#L157-L184)
with a more verbose finally block. Same thing, but shaves a few lines of code
off of the block. You don't gain much if you need to catch an exception, though
because you still need to put a try catch inside of the using block.
The using block does do one nice thing. Since it is a block, the variable
goes out of scope when it is done.
```c#
using (var context = searcherManager.AcquireContext)
{
var foo = context.Reference;
foo.Search();
} // foo goes out of scope
```
The only issue is that it is not 100% obvious that this is a problem:
```c#
Searcher foo;
using (var context = searcherManager.AcquireContext)
{
foo = context.Reference;
} // foo doesn't go out of scope - BAD
foo.Search(); // Not allowed here
```
But this just seems to be a scenario where there is no real way to force
the end user to follow the rules. You just have to document the right way and
hope they follow the documentation.
---
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.
---
