Closed plastikfan closed 4 years ago
I've just realised that all the manual normalisation that I've being doing turning arrays of things into a map, could have been achieved simply by using the R.indexBy function. Need to convert existing normalise code to use R.indexBy instead.
Some of the functionality in normalise can be composed with R.pipe as follows:
const e_names = R.pipe(
R.prop(groupName),
R.prop('_children'),
R.groupBy(R.prop(expressionId)),
R.map((item) => {
if (item.length > 1) {
throw new Error(`Expression(@${expressionId}): ${JSON.stringify(R.head(item)[expressionId])} already defined`);
}
return R.head(item);
})
)(expressionGroups);
However, subsequently we need access to one of the intermediate values that piping removes, so for now, this optimisation will not be made.
We need to have a new version of indexBy and groupBy which allows either the user to provide an addition function to pass in to handle duplicate items or simply to throw an error.
in this following code (not checked in):
const expressions = R.pipe(
R.prop(groupName),
R.prop('_children'),
R.indexBy(R.prop(expressionId)),
R.map((item) => {
if (item.length > 1) {
throw new Error(`Expression(@${expressionId}): ${JSON.stringify(R.head(item)[expressionId])} already defined`);
}
return R.head(item);
})
)(expressionGroups);
the only reason why we can't use the indexBy instead of groupBy, is that indexBy, hides the fact there many be duplicate items with the same key, whereas groupBy, will simply add multiple values for the same key into the resulting array, and we can simply detect the duplicate by checking the size of the array created for us is > 1. So indexBy, needs to give us a way to not handle duplicates silently.
The above comments on indexBy/groupBy were accidentally written on the wrong issue, was supposed to be on feat #6 (Add expression-builder)
Branch: express
Another bingo moment. I could have used reduceBy instead of groupBy, which resolves the issue of groupBy creating an array instead of a single item.
There are more than Expression/Pattern elements that can contain embedded regular expressions, eg there is Pattern/Yield/Placeholder. Therefore, the evaluate method has to be refactored so that expressions can be evaluated regardless of their location.
The expression builder needs to compose regular expressions from the Patterns list inside each Expression object. The result is still a map, but now the result of invoking the expression builder with a regular expression name should be a real regex. Actually, the Expression object may have other items on it, so the reguluar expression should be obtained via a property on the indexed expression, perhaps called "regex".