private final
Your fieldsdateStart
,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 returningboolean
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 includesIllegalArgumentException
See another answer of mine here here regarding such exceptions.
private final
Your fieldsdateStart
,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 returningboolean
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 includesIllegalArgumentException
See another answer of mine here regarding such exceptions.
private final
Your fieldsdateStart
,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 returningboolean
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 includesIllegalArgumentException
See another answer of mine here regarding such exceptions.
private final
Your fieldsdateStart
,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 returningboolean
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 includesIllegalArgumentException
See another answer of mine here regarding such exceptions.