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

Made PlcInvalidFieldException.java checked. #21

Closed

Conversation

JulianFeinauer
Copy link
Contributor

Made PlcInvalidFieldExeption checked, added UnchekedInvalidFieldException and changed everywhere.

Copy link
Contributor

@sruehl sruehl left a comment

Choose a reason for hiding this comment

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

We should discuss if this is the right way to go. In my opinion errors like these should always be runtime errors as theses a programming errors (e.g. ArrayIndexOutOfBoundException) and can't be handled properly at runtime so no need to check them. In contrast if this error could happen at runtime like a connection drop for reconnects etc. than it worth to enforce the catching of this exception so the developer can implement his own handling of this. But in this case in most cases the try catch wouldn't contain any useful code as the address ist unlikely to change at runtime (errors resulting in a parsing error).

(// TODO How should this be handled? might be exactly the point im trying to make ;))

@JulianFeinauer
Copy link
Contributor Author

Hi @sruehl I think I cannot answer to your question directly, I think.
I get your point and in these points you point out it's true that I do not know what the appropriate handling is.
I opened a dicussion thread on the list to exchange the arguments and find a common understanding :)

@JulianFeinauer
Copy link
Contributor Author

Hey @chrisdutz and @sruehl just wanted to bump that up. I think we should decide here on something and either merge or decline the PR. What do you hink about the checked exceptions for wrong user inputs?

@sruehl
Copy link
Contributor

sruehl commented Oct 2, 2018

As I said: The annoyance to be forced to catch a exception would outweigh the benefit. In the usual api use this would a one-time-occur-exception so we would require the developer to pollute the code with catch(e) new RuntimeException(e) snippets. So I would suggest/vote to close this PR and maybe pickup the topic in later releases (if at all).

@JulianFeinauer
Copy link
Contributor Author

Closing as suggested by sebastian.
But I definitly like to adress the topic nonetheless as we heavily depend on user input which can always be errorous.

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.

None yet

2 participants