lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From NightOwl888 <...@git.apache.org>
Subject [GitHub] lucenenet pull request #207: API: Added ReferenceManager<G>.AcquireContext()
Date Tue, 16 May 2017 17:12:35 GMT
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 <shad@shadstorhaug.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message