vorburger commented on a change in pull request #1019:
URL: https://github.com/apache/fineract/pull/1019#discussion_r443114391
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/ClientStatusChecker.java
##########
@@ -25,6 +25,12 @@
import org.slf4j.LoggerFactory;
public class ClientStatusChecker {
+
+ protected ClientStatusChecker() {
+ // prevents calls from subclass(if any)
+ throw new UnsupportedOperationException();
+ }
Review comment:
This is weird / un-usual... I think the typical solution for this is to
make the constructor private, and/or (?) declare the class to be `final` so
that there can be no subclasses.
```suggestion
private ClientStatusChecker() {
}
```
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/bulkimport/populator/client/ClientEntityWorkbookPopulatorTest.java
##########
@@ -60,9 +60,8 @@ public void setup(){
@Test
public void testClientEntityWorkbookPopulate() throws IOException {
//in order to populate helper sheets
- StaffHelper staffHelper=new StaffHelper();
requestSpec.header(HttpHeaders.CONTENT_TYPE,
MediaType.APPLICATION_JSON);
- Integer outcome_staff_creation
=staffHelper.createStaff(requestSpec,responseSpec);
+ Integer outcome_staff_creation
=StaffHelper.createStaff(requestSpec,responseSpec);
Review comment:
Question: Is this (static invocation of static methods) something you
have found a way to make your IDE do automatically? If yes, then this would
even allow us to enable Error Prone's `MethodCanBeStatic` in FINERACT-822...
but doing it manually for 301 cases (according to @percyashu) is probably not
"worth it" (https://en.wikipedia.org/wiki/Diminishing_returns).
##########
File path:
fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/bulkimport/populator/loan/LoanWorkbookPopulatorTest.java
##########
@@ -64,18 +64,15 @@ public void testLoanWorkbookPopulate() throws IOException {
Assert.assertNotNull("Could not create office"
,outcome_office_creation);
//in order to populate helper sheets
- ClientHelper clientHelper=new ClientHelper(requestSpec,responseSpec);
- Integer
outcome_client_creation=clientHelper.createClient(requestSpec,responseSpec);
Review comment:
this is actually a very nice clean up - as it makes absolutely no sense
to pass requestSpec, responseSpec twice
----------------------------------------------------------------
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:
[email protected]
