fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] awasum commented on a change in pull request #1022: FINERACT-822 enable UnnecessaryDefaultInEnumSwitch error
Date Fri, 12 Jun 2020 15:49:36 GMT

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



Mime
View raw message