vidakovic commented on code in PR #5856:
URL: https://github.com/apache/fineract/pull/5856#discussion_r3260388727


##########
fineract-loan/src/main/java/org/apache/fineract/portfolio/collateralmanagement/domain/CollateralManagementDomain.java:
##########
@@ -77,53 +77,40 @@ private CollateralManagementDomain(final String quality, 
final BigDecimal basePr
         this.name = name;
     }
 
-    public static CollateralManagementDomain createNew(JsonCommand 
jsonCommand, final ApplicationCurrency applicationCurrency) {
-        String quality = jsonCommand.stringValueOfParameterNamed("quality");
-        BigDecimal basePrice = 
jsonCommand.bigDecimalValueOfParameterNamed("basePrice");
-        BigDecimal pctToBase = 
jsonCommand.bigDecimalValueOfParameterNamedDefaultToNullIfZero("pctToBase");
-        String unitType = jsonCommand.stringValueOfParameterNamed("unitType");
-        String name = jsonCommand.stringValueOfParameterNamed("name");
+    public static CollateralManagementDomain createNew(final String name, 
final String quality, final BigDecimal basePrice,

Review Comment:
   We have these Lombok builders... let's use them. These static functions as a 
replacement for constructors (which they are not) are not so great for 
understanding what's happening behind the scenes. And in most of the cases 
these functions are used only in 1 place. In short: let's try to get rid of 
this function, domain classes should only contain attributes and getter/setter 
(via Lombok annotations).



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to