Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Exception messages when working on "leader" #573

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

TobiasNx
Copy link
Contributor

@TobiasNx TobiasNx commented Nov 19, 2024

Add leader hint to error message. See #573

Open adjust the message:

throw new IllegalStateException("only top level entities are allowed");

throw new AssertionError("unknown or unexpected state: " + state);

Also it would be greate to hint, which leader position or position name which is wrong.

@TobiasNx TobiasNx requested a review from dr0i November 19, 2024 08:27
Copy link
Member

@dr0i dr0i left a comment

Choose a reason for hiding this comment

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

Beside the one mentioned other messages are OK.

@@ -278,7 +278,7 @@ private void requireValidCode(final char code, final char[] validCodes) {
return;
}
}
throw new FormatException("invalid code '" + code + "'; allowed codes are: " + Arrays.toString(validCodes));
throw new FormatException("invalid code in leader'" + code + "'; allowed codes are: " + Arrays.toString(validCodes));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dr0i is it possible to add the info about the leader element? like this:

Suggested change
throw new FormatException("invalid code in leader'" + code + "'; allowed codes are: " + Arrays.toString(validCodes));
throw new FormatException("invalid code in leader literal literal'" + name + code + "'; allowed codes are: " + Arrays.toString(validCodes));

Copy link
Member

Choose a reason for hiding this comment

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

You've proposed to add literal literal as message. And a variable name which is not declared in the method nor globally (meaning: you cannot use it here).
What you probably meant to output was something like e.g. TYPE_OF_CONTROL_CODES or ENCODING_LEVEL_CODES so that a librarian knows what leader position is wrong? We could do this, but have to pass this information somehow into the method via a parameter.

@TobiasNx TobiasNx assigned dr0i and unassigned TobiasNx Nov 19, 2024
@dr0i dr0i assigned TobiasNx and unassigned dr0i Nov 19, 2024
@dr0i dr0i changed the title Update Marc21Encoder.java Improve Exception messages when working on "leader" Nov 19, 2024
@dr0i
Copy link
Member

dr0i commented Nov 19, 2024

Note: don't use titles like Update Marc21Encoder.java.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

2 participants