Skip to content

Conversation

mlopezFC
Copy link
Member

Creating the PR but it should be rebased after #69 is merged.

Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#69 was merged, please rebase

Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the error message be "Problem when trying to obtain id of entity of type %s ...", entity.getClass().getName()? While getId can fail for a specific entity, the most likely cause is that the method doesn't exist.

The semantics of IllegalStateException do not fit this description, because the error is not related to any state: (IllegalStateException "signals that a method has been invoked at an illegal or inappropriate time. In other words, the Java environment or Java application is not in an appropriate state for the requested operation"). It isn't an IllegalArgumentException either (as explained above, the most likely cause is not related to the argument, but to the lack of a getId method, and any entity would fail). I would just go with UndeclaredThrowableException.

Regardless of which exception is thrown, the actual cause must be passed to the exception constructor, so that the stack trace contains enough information about what failed.

@javier-godoy javier-godoy merged commit 5e0045f into master Jun 26, 2023
@javier-godoy javier-godoy deleted the issue-59 branch June 26, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants