bipinprasad commented on a change in pull request #3274:
URL: https://github.com/apache/storm/pull/3274#discussion_r435267750
##########
File path: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
##########
@@ -814,7 +814,12 @@ private static int numUsedWorkers(SchedulerAssignment
assignment) {
private boolean auditAssignmentChanges(Map<String, Assignment>
existingAssignments,
Map<String, Assignment>
newAssignments) {
- assert existingAssignments != null && newAssignments != null;
+ if (existingAssignments == null) {
Review comment:
Throwing unchecked exceptions is generally not considered good practice.
So using Preconditions would be not be considered good practice. Unchecked
Exceptions should be thrown only in rare cases. In this case, it is a private
method, totally in control of the class scope, and therefore no reason to throw
unchecked exceptions.
With regard to checking for null - that will not be considered defensive
programming. Defense would be to check for null and then do something about it
- i.e. set to to an empty list instead. The idea being that the method can
handle parameters can can be "bad" in some sense. In this specific private
method, that defense is not required.
Throwing unchecked exception is not a defense.
last few lines here:
https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
