-
Notifications
You must be signed in to change notification settings - Fork 0
adds substationOrVoltageLevelFilters #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
| Map<EquipmentType, List<String>> result = new EnumMap<>(EquipmentType.class); | ||
|
|
||
| List<AbstractFilter> genericFilters = null; | ||
| List<AbstractFilter> genericFilters = List.of(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| List<AbstractFilter> genericFilters = List.of(); | |
| List<AbstractFilter> genericFilters = new ArrayList<>(); |
is it better ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed here : e344a08
| genericFilters = filterLoader.getFilters(globalFilter.getGenericFilter()); | ||
| } | ||
|
|
||
| List<AbstractFilter> substationOrVoltageLevelFilters = List.of(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed here : e344a08
| */ | ||
| public static boolean shouldProcessEquipmentType(@Nonnull final EquipmentType equipmentType, | ||
| @Nonnull final List<AbstractFilter>genericFilters) { | ||
| @Nonnull final List<AbstractFilter> genericFilters) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strange sonar did not see that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed.
| final List<String> filteredIds = applyGlobalFilterOnNetwork( | ||
| network, | ||
| globalFilter, | ||
| equipmentType, | ||
| genericFilters, | ||
| substationOrVoltageLevelFilters, | ||
| filterLoader | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small refactoring suggestion : dont send globalFilter but only what we need, because we have genericFilters and substationOrVoltageLevelFilters accessibles from two different ways after
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is not a good thing to have data accessibles from two different ways. This was discussed last month and the genericFilters "cache" was kept.
Your idea sounds good but globalFilter contains a lot of things and most are used later in :
public static ExpertFilter buildExpertFilter(@Nonnull final GlobalFilter globalFilter,
@Nonnull final EquipmentType equipmentType,
@Nonnull final List<AbstractFilter> genericFilters,
@Nonnull final List<AbstractFilter> substationOrVoltageLevelFilters) {
So we would end with a lot of function parameters which is not much better in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think only the other filters are used later. You add 3 params, remove 1. So 2 more params. Definetely worth it IMO, comparing to the fact of having the variable name duplicated and containing two different things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But well if you already discussed about it, then I guess it's already chosen
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
Signed-off-by: Mathieu DEHARBE <[email protected]>
|



In the global filters :
Separates voltage level and substation generic filters from the others. (they are now a category in itself)