Source code
package org.apache.kafka.controller.errors;
import org.apache.kafka.common.errors.NotControllerException;
import org.apache.kafka.common.errors.TimeoutException;
import java.util.OptionalInt;
import java.util.concurrent.ExecutionException;
public class ControllerExceptions {
/**
* Check if an exception is a normal timeout exception.
*
* @param exception The exception to check.
* @return True if the exception is a timeout exception.
*/
public static boolean isTimeoutException(Throwable exception) {
if (!((exception == null))) {} else { return false; }
if (!((exception instanceof ExecutionException))) {} else {
exception = exception.getCause();
if (exception == null) return false;
}
return exception instanceof TimeoutException;
}
/**
* Create a new exception indicating that the controller is in pre-migration mode, so the
* operation cannot be completed.
*
* @param controllerId The current controller.
* @return The new NotControllerException.
*/
public static NotControllerException newPreMigrationException(OptionalInt controllerId) {
if (!((controllerId.isPresent()))) {
return new NotControllerException("No controller appears to be active.");
} else {
return new NotControllerException("The controller is in pre-migration mode.");
}
}
/**
* Create a new exception indicating that current node is not the controller.
*
* @param controllerId The current controller.
* @return The new NotControllerException.
*/
public static NotControllerException newWrongControllerException(OptionalInt controllerId) {
if (!((controllerId.isPresent()))) {
return new NotControllerException("No controller appears to be active.");
} else {
return new NotControllerException("The active controller appears to be node " +
controllerId.getAsInt() + ".");
}
}
}
Run 1
The code is generally readable but contains unusual constructs that impair clarity. In `isTimeoutException`, the pattern `if (!((exception == null))) {} else { return false; }` is a convoluted way to write `if (exception == null) return false;`, which adds confusion. Similarly, the check for `ExecutionException` uses an empty if-block with logic in the else, violating the principle of clear intent. The other methods are straightforward and well-documented, with concise logic and good action granularity. However, `newPreMigrationException` does not use its `controllerId` parameter, which is ambiguous. Overall structure is logical and nesting is minimal, but the first method's unnatural control flow and unused parameter reduce readability.
Run 2
The code structure is generally logical, with clear method separation and helpful Javadoc comments. However, readability is severely impacted by the use of inverted conditional patterns like 'if (!((condition))) {} else { ... }' in isTimeoutException. This is a confusing anti-pattern that obscures intent and makes the code difficult to follow. Nesting depth is moderate, but the awkward conditional logic creates unnecessary cognitive overhead. While variable names are reasonable and methods are concise, the abnormal syntax undermines clarity. The action granularity is acceptable, but reading flow is broken by the need to mentally invert conditions. The controller ID parameter is unused in the last two methods, adding slight ambiguity. Overall, the code works but requires extra effort to parse due to unconventional conditional constructs.
Run 3
The code has clear documentation and logical purpose, but suffers from awkward control flow. The `if (!((condition))) {} else { ... }` pattern is highly unidiomatic and confusing; standard `if (condition) { ... }` would be much clearer. In `isTimeoutException`, the null check and exception unwrapping are hard to follow due to this style. The helper methods for exceptions are straightforward, but there is minor redundancy: both `newPreMigrationException` and `newWrongControllerException` share the 'No controller appears to be active' branch, which could be extracted. The string concatenation in `newWrongControllerException` is acceptable. Overall, the code is readable at a high level due to comments and simple actions, but the unnatural conditionals reduce clarity and maintainability.
Run 1
Code Structure: Three clearly separated utility methods provide logical organization. Nesting: Depth is low, but empty if branches and unnecessary else blocks obscure otherwise simple checks. Clarity of intent: Descriptive method names, Javadoc, and exception messages explain the purpose; negated conditions and redundant parentheses make the implementation harder to interpret. Code length: The class is reasonably short, though empty branches and avoidable syntax add noise. Action granularity: Most statements perform one action, but inline empty branches and returns compress control flow onto single lines. Reading flow: Methods follow a sensible sequence, yet inverted checks repeatedly interrupt natural reading. Straightforward null checks, positive instanceof conditions, and early returns would substantially improve readability.
Run 2
Code Structure: Three small, logically separated methods provide good organization. Nesting: Generally shallow, although empty branches and else blocks introduce unnecessary complexity. Clarity of intent: Descriptive method names, documentation, and exception messages explain the purpose, but negated conditions with redundant parentheses obscure straightforward checks. Code length: Compact overall, with avoidable empty branches and else blocks after returns. Action granularity: Most statements perform one action, but inline conditionals and returns in isTimeoutException reduce scanability. Reading flow: Method boundaries and documentation support natural reading, while inverted conditions and empty if bodies repeatedly interrupt it. Conventional guard clauses and simpler boolean expressions would substantially improve readability.
Run 3
Code Structure: Three small, focused methods provide logical organization, although empty if branches obscure the timeout-checking logic. Nesting: Mostly shallow, with only one nested null check. Clarity of intent: Descriptive method names, documentation, and exception messages explain the purpose, but negated conditions and redundant parentheses make simple checks harder to interpret. Code length: Compact overall, though empty branches and unnecessary else blocks add avoidable syntax. Action granularity: Most lines contain one action, but inline conditionals and the compressed empty-branch/else statements reduce scanability. Reading flow: Method boundaries and documentation support navigation, while inverted control flow interrupts the natural progression of guard checks and exception handling.