marta-jankovics commented on code in PR #3563:
URL: https://github.com/apache/fineract/pull/3563#discussion_r1442398971
##########
fineract-core/src/main/java/org/apache/fineract/accounting/glaccount/domain/GLAccountType.java:
##########
@@ -86,9 +82,25 @@ public static EnumOptionData fromString(String accountType) {
}
}
- public static GLAccountType fromInt(final int i) {
- final GLAccountType type = intToEnumMap.get(Integer.valueOf(i));
- return type;
+ public static GLAccountType fromInt(final Integer v) {
Review Comment:
Can you please not hardcode the mapping but do something dynamic, which
works even after a new enum element is added.
Usually I do something like in SavingsAccountTransactionType
private static final Map<Integer, SavingsAccountTransactionType> BY_ID =
Arrays.stream(VALUES).collect(Collectors.toMap(SavingsAccountTransactionType::getValue,
v -> v));
public static SavingsAccountTransactionType fromInt(final Integer value)
{
SavingsAccountTransactionType transactionType = BY_ID.get(value);
return transactionType == null ? INVALID : transactionType;
}
##########
fineract-core/src/main/java/org/apache/fineract/accounting/journalentry/domain/JournalEntryType.java:
##########
@@ -41,16 +38,19 @@ public String getCode() {
return this.code;
}
- private static final Map<Integer, JournalEntryType> intToEnumMap = new
HashMap<>();
-
- static {
- for (final JournalEntryType type : JournalEntryType.values()) {
- intToEnumMap.put(type.value, type);
+ public static JournalEntryType fromInt(final Integer v) {
+ if (v == null) {
+ return null;
}
- }
- public static JournalEntryType fromInt(final int i) {
- return intToEnumMap.get(i);
+ switch (v) {
Review Comment:
Same here as for GLAccountType
##########
fineract-core/src/main/java/org/apache/fineract/accounting/producttoaccountmapping/domain/PortfolioProductType.java:
##########
@@ -47,33 +44,41 @@ public String getCode() {
return this.code;
}
- private static final Map<Integer, PortfolioProductType> intToEnumMap = new
HashMap<>();
-
- static {
- for (final PortfolioProductType type : PortfolioProductType.values()) {
- intToEnumMap.put(type.value, type);
+ public static PortfolioProductType fromInt(final Integer v) {
+ if (v == null) {
+ return null;
}
- }
- public static PortfolioProductType fromInt(final int i) {
- final PortfolioProductType type = intToEnumMap.get(Integer.valueOf(i));
- return type;
+ switch (v) {
Review Comment:
Please do not hardcode the mapping, same as for GLAccountType. I do not
comment other enums but this request applies for all of them.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JavaType.java:
##########
@@ -91,9 +91,7 @@ public enum JavaType {
OBJECT(Object.class), //
;
- public static final JavaType[] VALUES = values();
-
- private static final Map<Class<?>, JavaType> BY_TYPE =
Arrays.stream(VALUES).filter(e -> e.type != null)
+ private static final Map<Class<?>, JavaType> BY_TYPE =
Arrays.stream(values()).filter(e -> e.type != null)
Review Comment:
Actually it does make sense to have a static VALUES array, because values()
is dynamic. If it only used here for the static mapper then you can remove, but
if it is used anywhere else then it worths to keep.
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JdbcJavaType.java:
##########
@@ -136,11 +136,19 @@ public static JdbcJavaType getByTypeName(@NotNull
DatabaseType dialect, String n
for (JdbcJavaType type : values()) {
DialectType dialectType = type.getDialectType(dialect);
if (dialectType.getNameResolved().equals(name)) {
+ // NOTE: make MySQL systems happy aka TINYINT vs BOOLEAN issue!
Review Comment:
Can you please explain the use-case why do you need this and why you do not
change the BOOLEAN and TINYINT behaviour for mysql?
##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/service/database/JdbcJavaType.java:
##########
@@ -136,11 +136,19 @@ public static JdbcJavaType getByTypeName(@NotNull
DatabaseType dialect, String n
for (JdbcJavaType type : values()) {
DialectType dialectType = type.getDialectType(dialect);
if (dialectType.getNameResolved().equals(name)) {
+ // NOTE: make MySQL systems happy aka TINYINT vs BOOLEAN issue!
+ if (type.canBooleanType(dialect)) {
+ return BOOLEAN;
+ }
return type;
}
if (dialectType.alterNames != null) {
for (String alterName : dialectType.alterNames) {
if (alterName.equals(name)) {
+ // NOTE: make MySQL systems happy aka TINYINT vs
BOOLEAN issue!
Review Comment:
Can you please explain the use-case why do you need this and why you do not
change the BOOLEAN and TINYINT behaviour for mysql?
--
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]