On 03/05/2015 02:10 AM, Ondrej Zizka wrote:
Hi,
1) WindupRuleMetadata should IMO be named RuleProvidersRegistry.
2) WindupRuleMetadata has:
private final List<WindupRuleProvider> providers = new ArrayList<>();
private final IdentityHashMap<WindupRuleProvider, List<Rule>>
providersToRules = new IdentityHashMap<>();
private Configuration configuration;
Isn't the first redundant? It can be acquired simply by
providersToRules.getKeys().
And WRT configuration - that one only has
public List<Rule> getRules();
Is that another redundancy, since rules are in the map? It could be
retrieved as merge of getValues().
The current metadata PR renames it to "LoadedRules". I think that I like
the name RuleProviderRegistry better than that, though.
The reason for multiple stores is the ordering. The Map is not an
ordered map (though it could probably be replaced with one that is). I'm
not really sure what would be gained by getting rid of the Configuration
object itself. That is ultimately what WindupProcessorImpl uses to
execute the rules. I guess that it could call getRules() directly and
create that later, but I'm not sure if that is really an improvement.