szetszwo commented on code in PR #1068:
URL: https://github.com/apache/ratis/pull/1068#discussion_r1571013114


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -1805,17 +1805,17 @@ TransactionContext getTransactionContext(LogEntryProto 
entry, Boolean createNew)
   CompletableFuture<Message> 
applyLogToStateMachine(ReferenceCountedObject<LogEntryProto> nextRef)
       throws RaftLogIOException {
     LogEntryProto next = nextRef.get();
-    if (!next.hasStateMachineLogEntry()) {
-      stateMachine.event().notifyTermIndexUpdated(next.getTerm(), 
next.getIndex());
-    }
-
-    if (next.hasConfigurationEntry()) {
+    switch (next.getLogEntryBodyCase()) {

Review Comment:
   It is good to use `switch`.  Let add also `case METADATAENTRY` and make 
`default` throw an exception.



##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -1805,17 +1805,17 @@ TransactionContext getTransactionContext(LogEntryProto 
entry, Boolean createNew)
   CompletableFuture<Message> 
applyLogToStateMachine(ReferenceCountedObject<LogEntryProto> nextRef)
       throws RaftLogIOException {
     LogEntryProto next = nextRef.get();
-    if (!next.hasStateMachineLogEntry()) {
-      stateMachine.event().notifyTermIndexUpdated(next.getTerm(), 
next.getIndex());
-    }
-
-    if (next.hasConfigurationEntry()) {
+    switch (next.getLogEntryBodyCase()) {
+    case CONFIGURATIONENTRY:
       // the reply should have already been set. only need to record
       // the new conf in the metadata file and notify the StateMachine.
       state.writeRaftConfiguration(next);
-      stateMachine.event().notifyConfigurationChanged(next.getTerm(), 
next.getIndex(), next.getConfigurationEntry());
+      stateMachine.event().notifyConfigurationChanged(next.getTerm(), 
next.getIndex(),
+          next.getConfigurationEntry());
       role.getLeaderState().ifPresent(leader -> leader.checkReady(next));
-    } else if (next.hasStateMachineLogEntry()) {
+      stateMachine.event().notifyTermIndexUpdated(next.getTerm(), 
next.getIndex());

Review Comment:
   Let move it down to right before `return null`.  Then, we don't have to 
repeat it for each case.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to