Oof, the lesson that should have been learned here is that this is an inherently unsafe way to design the API of this class. While Java's type system isn't the most expressive in the world, certainly you could design something that makes it impossible to construct in a way that you'd ever see this failure.<p>Hell, you could even use a builder pattern (and make the constructor private), which is pretty common in Java-land:<p><pre><code> final ErrorResponse errorResponse = ErrorResponse.newBuilder()
.withSuccesses(1, "Great!")
.withWarnings(3, "Hmm...")
.withErrors(1, "Uh oh")
.build();
</code></pre>
If you have no warnings (for example), you don't call `.withWarnings()`, and then you have zero warnings and no message to go with it. If you have warnings, you call the `.withWarnings()` method and you <i>must</i> provide both bits of information.<p>Yes, it's more verbose. But that's Java for you, and the key takeaway is that it's impossible to construct an `ErrorMessage` with invalid internal state (assume that the `.with*()` methods have proper preconditions, etc.).<p>In addition, `.getSuccessMsg()` etc. should return `Optional<String>`. Alternatively, you could just have `.getSuccesses()`, which returns something like `Optional<ResponseData>`, which is just a poor-man's 2-tuple of (numSuccesses, successMsg). (And if you're using a recent Java version [which I suspect Salesforce is not], you can even use a record to save you the boilerplate.)
> Just because one field tells you something should be there doesn’t mean an extra null-check would hurt.<p>IMO, it would. NPE exists for a reason: to instantly alert you to a bug and/or contract violation. Maybe I misunderstood the author's intent here, but sweeping the contract violation under the rug with a superfluous null check doesn't seem to help anything here. If successMsg is null, what are you going to do, display "" instead? Often, immediate termination with NPE seems to be the best approach in cases like this, IMHO.
Overloaded constructors where the parameter order is different in overloads doesn't seem like a great idea.<p>Surely<p>> public ErrorResponse(int successCount, int errorCount, String errorMsg, String warningMsg);<p>> public ErrorResponse(int successCount, int errorCount, String errorMsg, String warningMsg, String successMsg);<p>would cause fewer problems than<p>> public ErrorResponse(int successCount, int errorCount, String warningMsg, String errorMsg);<p>> public ErrorResponse(int successCount, int errorCount, String successMsg, String warningMsg, String errorMsg);
I am more surprised to see that Java didn't raise a compile time error for trying to read the value of an argument that doesn't exist in the method signature (successMsg).