> On 2011-09-21 18:13:45, Thomas Koch wrote: > > I don't think it's nice to keep adding Constructors to the ZooKeeper class. > > How about the builder pattern (see Joshua Blocks effective Java).
I'm not totally sure about how to implement it (http://javapapers.com/design-patterns/builder-pattern/) mainly because connection is started at constructor: "cnxn.start();" I agree with you that too many constructors could be a bit ugly so it could be good enough to replace all new constructors with only one like "public ZooKeeper(ZooCfg cfg) throws IOException". > On 2011-09-21 18:13:45, Thomas Koch wrote: > > /src/java/main/org/apache/zookeeper/ZooKeeper.java, line 433 > > <https://reviews.apache.org/r/1977/diff/1/?file=43783#file43783line433> > > > > no tabs please. Fixed. > On 2011-09-21 18:13:45, Thomas Koch wrote: > > /src/java/main/org/apache/zookeeper/ZooKeeper.java, line 430 > > <https://reviews.apache.org/r/1977/diff/1/?file=43783#file43783line430> > > > > throwing an exception changes the public API of ZooKeeper. This may not > > be possible in a point release. "throws IOException" is at line 431 on previous version. - César ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1977/#review2004 ----------------------------------------------------------- On 2011-09-20 09:10:45, César Álvarez Núñez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1977/ > ----------------------------------------------------------- > > (Updated 2011-09-20 09:10:45) > > > Review request for zookeeper. > > > Summary > ------- > > . > > > This addresses bug https://issues.apache.org/jira/browse/ZOOKEEPER-1172. > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/ZOOKEEPER-1172 > > > Diffs > ----- > > /src/java/main/org/apache/zookeeper/ZooKeeper.java 1169669 > /src/java/main/org/apache/zookeeper/client/ConnectStringParser.java 1169669 > /src/java/main/org/apache/zookeeper/client/HostProvider.java 1169669 > /src/java/main/org/apache/zookeeper/client/StaticHostProvider.java 1169669 > /src/java/test/org/apache/zookeeper/test/ConnectStringParserTest.java > 1169669 > /src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java > 1169669 > > Diff: https://reviews.apache.org/r/1977/diff > > > Testing > ------- > > > Thanks, > > César > >
