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

Change InvalidRecurrenceRuleException to be a RuntimeException #30

Open
roben opened this issue May 4, 2017 · 3 comments
Open

Change InvalidRecurrenceRuleException to be a RuntimeException #30

roben opened this issue May 4, 2017 · 3 comments

Comments

@roben
Copy link

roben commented May 4, 2017

It would be great if InvalidRecurrenceRuleException could be changed to be a RuntimeException so it does not have to be handled explicitly every time a rule String is parsed. I use validators before processing them, so an invalid rule is an application error in my case.

@roben roben changed the title Change InvalidRecurrenceRuleException to RuntimeException Change InvalidRecurrenceRuleException to be a RuntimeException May 4, 2017
@roben
Copy link
Author

roben commented May 4, 2017

To clarify, code like this:

public Optional<RecurrenceRule> getParsedRule() {
    if (StringUtil.isNullOrEmpty(rule)) {
        return Optional.empty();
    } else {
        try {
            return Optional.of(new RecurrenceRule(rule));
        } catch (InvalidRecurrenceRuleException ex) {
            throw new RuntimeException(ex);
        }
    }
}

could become just this:

public Optional<RecurrenceRule> getParsedRule() {
    if (StringUtil.isNullOrEmpty(rule)) {
        return Optional.empty();
    } else {
        return Optional.of(new RecurrenceRule(rule));
    }
}

@dmfs
Copy link
Owner

dmfs commented May 4, 2017

Actually, I think the way you do it now is the better (I'm tempted to say the "correct") one.

For you it's a runtime error if you have an invalid RRULE at this point. For others who don't validate the RRULE in advance (like we do in our projects) a RuntimeException (or any subtype) would be inappropriate.

To be more specific, I want the Exception to be checked, so users of this class are aware that the constructor will throw if the rule is invalid.

We will, however, consider your use case when we revisit the overall design of this library.

Btw, if you validate the rule in advance you probably should explicitly specify which RfcMode you want to use, to make sure it matches your own validation.

@roben
Copy link
Author

roben commented May 4, 2017

I think checked Exceptions are an anti pattern but I know that there are also good arguments for the contrary and I understand your reasoning. Thanks for considering this for the redesign. Maybe there is a way for this without using any kind of throwable.

Also thanks for the advice with the RfcMode.

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

No branches or pull requests

2 participants