Revision b2783da1-2ae9-4432-bcff-aa61b7ed4860 - Code Review Stack Exchange
- `private final` Your fields `dateStart`, `dateStop` and the others should be marked as **private final**. Other classes don't need direct access to them (and normally shouldn't have direct access even if they needed it), and as they never change once the class has been initialized it's good practice to mark them as **final**.
- I'm not a big fan of the lack of line breaks in your `switch` statements. Also, by returning `boolean` as has been suggested in other answers, you can simplify it to: (Note the improved formatting)
switch (repetitionType) {
case WEEKLY:
return checkWeekRepetition(dateToCheck);
case MONTHLY:
return checkMonthRepetition(dateToCheck);
}
I also removed the `if (repetitionType != null)` check as that's not needed, if it would be null the default case would apply, and as your default case was just empty there's no need in having it at all.
- `catch (IllegalArgumentException ex) {`
You catch the IllegalArgumentException to use it as flow-control by returning `false` if it occurs. Don't do that. Don't use Exceptions for flow-control in the first place, and I also have to point out that some exceptions are not meant to be caught. That includes `IllegalArgumentException` See another answer of mine [here](https://codereview.stackexchange.com/questions/45637/exception-handling-with-instanceof-rather-than-catch/45639#45639) regarding such exceptions.