Pass message to IntegrityException#6949
Conversation
|
|
||
| class IntegrityException extends RuntimeException {} | ||
| class IntegrityException extends RuntimeException { | ||
| public IntegrityException(String message) { |
There was a problem hiding this comment.
What about making this nullable and optional so you don't need to pass "" in some of the call sites?
There was a problem hiding this comment.
Making the message nullable wouldn’t simplify things in this case. This is plain Java code, so I’d still need to pass null explicitly or add an extra constructor to handle the no-arg case.
Also, "" is only used in tests, not in production code. In actual business logic, the exception always carries a meaningful message, and making the parameter nullable would rather increase the risk of accidentally passing null instead of a proper message.
There was a problem hiding this comment.
Good points! Probably not worth converting to Kotlin just for this change.
|
|
||
| class IntegrityException extends RuntimeException {} | ||
| class IntegrityException extends RuntimeException { | ||
| public IntegrityException(String message) { |
There was a problem hiding this comment.
Good points! Probably not worth converting to Kotlin just for this change.
Why is this the best possible solution? Were any other approaches considered?
The
IntegrityExceptionshould include the original exception message to provide more details about what happened.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
It doesn't require testing.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest