Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============4650708249002651040=="
MIME-Version: 1.0
Subject: Re: Review Request 19409: ACCUMULO-2497 - InstanceFactory
From: "Bill Havanki" <bhavanki@clouderagovt.com>
To: "John Vines" <vines@apache.org>,
 "Bill Havanki" <bhavanki@clouderagovt.com>,
 "accumulo" <dev@accumulo.apache.org>, "Sean Busbey" <sean@manvsbeard.com>,
 "Mike Drob" <mdrob@mdrob.com>
Date: Wed, 19 Mar 2014 19:34:13 -0000
Message-ID: <20140319193413.6288.82530@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Havanki" <noreply@reviews.apache.org>
References: <20140319165238.6335.52370@reviews.apache.org>
In-Reply-To: <20140319165238.6335.52370@reviews.apache.org>
Reply-To: "Bill Havanki" <bhavanki@clouderagovt.com>

--===============4650708249002651040==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> On March 19, 2014, 12:52 p.m., Mike Drob wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/InstanceFactory.java, line 37
> > <https://reviews.apache.org/r/19409/diff/1/?file=528072#file528072line37>
> >
> >     Is this exception class used? If not, it should be removed.
> 
> Bill Havanki wrote:
>     It's not, but I wanted to leave a way for future factory implementations to predictably throw exceptions, at the factory level of abstraction (Effective Java 2nd ed. Item 61).
> 
> John Vines wrote:
>     I like that. Could we add a basic coding to the exception type to make various error cases easier to add.
>     
>     And is there a particular reason you're making that exception type extend Runtime instead of just Exception?
> 
> Sean Busbey wrote:
>     I presume it's because if it was not a RuntimeException it would have to go in the method signature. Since the class isn't public, it would have to throw Exception, which is not good.

The InstanceFactoryException class is in fact public because all members of interfaces are public (JLS 6.6.1, also I had to double-check with javap :) ). I definitely intend for it to be public.

I made it a RuntimeException because I judged that recovering from failing to get an Instance would be unrecoverable. I could be wrong, and if so, I'm happy to make it checked.

John, is there any particular technique for adding a coding that you like? Numeric code? Enum? I'll check around for any existing patterns for this sort of thing.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19409/#review37731
-----------------------------------------------------------


On March 19, 2014, 12:44 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19409/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 12:44 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-2497
>     https://issues.apache.org/jira/browse/ACCUMULO-2497
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> To help in testing, this commit adds an InstanceFactory interface with some implementations.
> 
> To avoid changing the public API, some classes are in o.a.a.core.client.impl.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/InstanceFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ZooKeeperInstanceFactory.java PRE-CREATION 
>   core/src/test/java/org/apache/accumulo/core/client/impl/ZooKeeperInstanceFactoryTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstanceFactory.java PRE-CREATION 
>   server/base/src/test/java/org/apache/accumulo/server/client/HdfsZooInstanceFactoryTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19409/diff/
> 
> 
> Testing
> -------
> 
> Unit tests pass.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


--===============4650708249002651040==--