> On Jan. 23, 2013, 7:20 a.m., Edward Ribeiro wrote: > > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java, line > > 467 > > <https://reviews.apache.org/r/6707/diff/4/?file=231478#file231478line467> > > > > Lines 467-479 contains duplicate code. I suggest to create a method > > split (or parse) method like: > > > > List<String> split(String input) { > > > > if (input != null) { > > > > List<String> list = Arrays.asList(input.split(",")); > > > > return Collections.unmodifiableList(list); > > } > > > > return Collections.emptyList(); > > } > > > > Then lines 467 and 474 can be simplified as: > > > > List<String> joiningServers = > > split(reconfigRequest.getJoiningServers()); > > > > List<String> leavingServers = > > split(reconfigRequest.getLeavingServers()); > > > > PS: I think this split method should go in a helper class as stated > > before.
need to additionally check that reconfigRequest.getJoiningServers() or reconfigRequest.getLeavingServers() return a non-null value. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6707/#review15611 ----------------------------------------------------------- On Nov. 29, 2012, 7:12 a.m., Alexander Shraer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6707/ > ----------------------------------------------------------- > > (Updated Nov. 29, 2012, 7:12 a.m.) > > > Review request for zookeeper. > > > Description > ------- > > see https://issues.apache.org/jira/browse/ZOOKEEPER-107 > > > Diffs > ----- > > /src/c/include/proto.h 1415037 > /src/c/include/zookeeper.h 1415037 > /src/c/src/cli.c 1415037 > /src/c/src/zookeeper.c 1415037 > /src/java/main/org/apache/zookeeper/KeeperException.java 1415037 > /src/java/main/org/apache/zookeeper/ZooDefs.java 1415037 > /src/java/main/org/apache/zookeeper/ZooKeeper.java 1415037 > /src/java/main/org/apache/zookeeper/ZooKeeperMain.java 1415037 > /src/java/main/org/apache/zookeeper/cli/GetConfigCommand.java PRE-CREATION > /src/java/main/org/apache/zookeeper/cli/ReconfigCommand.java PRE-CREATION > /src/java/main/org/apache/zookeeper/server/DataTree.java 1415037 > /src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > 1415037 > /src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > 1415037 > /src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 1415037 > /src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java > 1415037 > /src/java/main/org/apache/zookeeper/server/Request.java 1415037 > /src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java 1415037 > /src/java/main/org/apache/zookeeper/server/TraceFormatter.java 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/CommitProcessor.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/Follower.java 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/FollowerRequestProcessor.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/Leader.java 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/Learner.java 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/Observer.java 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/ObserverRequestProcessor.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumBean.java 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java > 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyRequestProcessor.java > 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumHierarchical.java > 1415037 > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java > 1415037 > > /src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumVerifier.java > 1415037 > /src/java/main/org/apache/zookeeper/server/util/ConfigUtils.java > PRE-CREATION > /src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java 1415037 > /src/java/test/org/apache/zookeeper/server/TruncateCorruptionTest.java > 1415037 > /src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 1415037 > /src/java/test/org/apache/zookeeper/server/quorum/ReconfigRecoveryTest.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java 1415037 > /src/java/test/org/apache/zookeeper/test/CnxManagerTest.java 1415037 > /src/java/test/org/apache/zookeeper/test/QuorumMajorityTest.java > PRE-CREATION > /src/java/test/org/apache/zookeeper/test/QuorumUtil.java 1415037 > /src/java/test/org/apache/zookeeper/test/ReconfigTest.java PRE-CREATION > /src/zookeeper.jute 1415037 > > Diff: https://reviews.apache.org/r/6707/diff/ > > > Testing > ------- > > New test files were added: ReconfigTest, ReconfigRecoveryTest and > QuorumMajorityTest. Many other tests were modified. > > > Thanks, > > Alexander Shraer > >
