You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/06/12 15:49:36 UTC

[GitHub] [fineract] awasum commented on a change in pull request #1022: FINERACT-822 enable UnnecessaryDefaultInEnumSwitch error

awasum commented on a change in pull request #1022:
URL: https://github.com/apache/fineract/pull/1022#discussion_r439499975



##########
File path: fineract-provider/src/main/java/org/apache/fineract/adhocquery/service/AdHocScheduledJobRunnerServiceImpl.java
##########
@@ -88,10 +88,8 @@ public void generateClientSchedule() {
                                 next = start.plusDays((int) (long) adhoc.getReportRunEvery());
                                 run = Days.daysBetween(start, end).getDays() >= adhoc.getReportRunEvery();
                                 break;
-                            default:
-                                throw new IllegalStateException();
                         }
-
+                        throw new IllegalStateException();

Review comment:
       @percyashu ..Maybe remove this exception? and put it back in the default case inside the } on line 91 above?
   
   Also if you want to add a default step, you need to check the `ReportRunFrequency.fromId(adhoc.getReportRunFrequency()` method from line 70 above and see how many different cases needs to be treated, then try to handle them all. Do we absolutely need a default case in all implementations of Switch statement?
   As seen here: https://errorprone.info/bugpattern/UnnecessaryDefaultInEnumSwitch, dont put a default statement if all the cases have been handled. Maybe skip it? Will that throw an error?
   
   Or given that ReportRunFrequency is an enum and we/Fineract is already handling all the 5 cases here, we can just throw a `new AdHocScheduledJobFrequencyException()` error here as default case with a message like "Correct Frequency not specified, Check the API docs"... @vorburger  @ptuomola your take?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org