Fix link variables #380
Fix link variables #380
Conversation
Hajarel-moukh
requested review from
alexisszmundy and
loichenninger
and removed request for
loichenninger
| // idem: on garde convertOneVar(entry, String, ...) | ||
| convertOneVar(collectedVariable, getValueString(value), variablesMap, 1, dest); | ||
| } | ||
| if (value != null && !(value instanceof List<?>)) { |
There was a problem hiding this comment.
issue: value instanceof List<?> is already checked in the if just before, hence it will always be false
suggestion: extract both ifs into a method and add return to the first if, then remove the !value instance of check
There was a problem hiding this comment.
@alicela would it be better if we extract into a method ?
| variablesMap, | ||
| dstSurveyUnitModel | ||
| ); | ||
| continue; |
There was a problem hiding this comment.
note: Sonar doesn't like more than one continue in a method
There was a problem hiding this comment.
I propose to a extract a method like this, that excludes all "continue" and is easier to read
private static void processCollectedVariable(
Map.Entry<String, Object> entry,
String stateKey,
VariablesMap variablesMap,
SurveyUnitModel dstSurveyUnitModel,
CollectedVariables dest
) {
if (Constants.LIENS.equals(entry.getKey())) {
handleLiensCollectedVariable(entry, DataState.valueOf(stateKey), variablesMap, dstSurveyUnitModel);
return;
}
Map<String, Object> states = JsonUtils.asMap(entry.getValue());
if (states == null) return;
Object value = states.get(stateKey);
if (value == null) return;
if (value instanceof List<?> list) {
convertListVar(list, entry, variablesMap, dest);
} else {
convertOneVar(entry, getValueString(value), variablesMap, 1, dest);
}
}
| .getVariable(Constants.LIEN + 1) | ||
| .getGroupName(); | ||
|
|
||
| for (int i = 0; i < individus.size(); i++) { |
There was a problem hiding this comment.
nitpick: Not a big fan of "i" or "k" as variable names as they mean nothing and we can get lost as we go deeper in the code
| } | ||
| private static VariableModel buildLienVariable( | ||
| List<String> liensIndividu, | ||
| int k, |
There was a problem hiding this comment.
suggestion: change "k" variable name to have a meaning behind it
| public static final int VOLUMETRY_FILE_EXPIRATION_DAYS = 30; | ||
|
|
||
| public static final int MAX_LINKS_ALLOWED = 21; | ||
| public static final String LIEN = "LIEN_"; |
|
|
||
| public static final int MAX_LINKS_ALLOWED = 21; | ||
| public static final String LIEN = "LIEN_"; | ||
| public static final String LIENS = "LIENS"; |
| variablesMap, | ||
| dstSurveyUnitModel | ||
| ); | ||
| continue; |
There was a problem hiding this comment.
I propose to a extract a method like this, that excludes all "continue" and is easier to read
private static void processCollectedVariable(
Map.Entry<String, Object> entry,
String stateKey,
VariablesMap variablesMap,
SurveyUnitModel dstSurveyUnitModel,
CollectedVariables dest
) {
if (Constants.LIENS.equals(entry.getKey())) {
handleLiensCollectedVariable(entry, DataState.valueOf(stateKey), variablesMap, dstSurveyUnitModel);
return;
}
Map<String, Object> states = JsonUtils.asMap(entry.getValue());
if (states == null) return;
Object value = states.get(stateKey);
if (value == null) return;
if (value instanceof List<?> list) {
convertListVar(list, entry, variablesMap, dest);
} else {
convertOneVar(entry, getValueString(value), variablesMap, 1, dest);
}
}
| dstSurveyUnitModel | ||
| ); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I think this method can be extract as in LunaticJsonRawDataService
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| static void handleLiensCollectedVariable( |
There was a problem hiding this comment.
change Liens into pairwise
| ) { | ||
| Object value = getValueForState(collectedVariable, dataState.toString()); | ||
|
|
||
| if (!(value instanceof List<?> individus) |
There was a problem hiding this comment.
add a boolean method to explain why you do these test
|
|
||
| for (int linkIndex = 1; linkIndex <= Constants.MAX_LINKS_ALLOWED; linkIndex++) { | ||
| dstSurveyUnitModel.getCollectedVariables().add( | ||
| buildLienVariable(individualLinks, linkIndex, individualIndex, groupName) |
There was a problem hiding this comment.
here is "lien" and "link".
This PR adds the processing logic for collected variables (LIENS) in Genesis.