Case sensitive BulkExpressionSolver#solve fails for capitalized keys declared in imperfect order
brandoncc opened this issue · 1 comments
Hello, and thanks for this awesome library!
I've built a system which uses primarily capitalized tokens in expressions. These expressions can reference other tokens with no limit to depth of references. To support this, I've created a graph generator that walks the formulas and has a way to output a list of all tokens found and their formulas, like this:
expressions = {
FIRST: "SECOND * 2",
SECOND: "THIRD * 2",
THIRD: 2,
}
When my formulas are more than one reference level deep, as the example above is, Calculator#solve!
raises an UnboundVariableError
.
I found that this is because my expression hash is in an order where tokens are referenced before they are declared. The hash above is an example of this. Here is a failing test that I've written in spec/bulk_expression_solver_spec.rb
to show the problem:
it 'resolves capitalized keys when they are declared out of order' do
expressions = {
FIRST: "SECOND * 2",
SECOND: "THIRD * 2",
THIRD: 2,
}
result = described_class.new(expressions, calculator).solve
expect(result).to eq(
FIRST: 8,
SECOND: 4,
THIRD: 2
)
end
The failure message is:
|| Failures:
||
|| 1) Dentaku::BulkExpressionSolver#solve resolves capitalized keys when they are declared out of order
|| Failure/Error:
|| expect(result).to eq(
|| FIRST: 8,
|| SECOND: 4,
|| THIRD: 2
|| )
||
|| expected: {:FIRST=>8, :SECOND=>4, :THIRD=>2}
|| got: {:FIRST=>:undefined, :SECOND=>4, :THIRD=>2}
||
|| (compared using ==)
||
|| Diff:
|| @@ -1 +1 @@
|| -:FIRST => 8,
|| +:FIRST => :undefined,
||
|| # ./spec/bulk_expression_solver_spec.rb:210:in `block (3 levels) in <top (required)>'
If I swap the order of FIRST and SECOND in expressions
, though, the test passes.
I dug into BulkExpressionSolver#variables_in_resolve_order
as a starting point. When the keys are in the order which allows the test to pass (SECOND, FIRST, THIRD), DependencyResolver.find_resolve_order(dependencies)
returns ["third", "SECOND", "second", "FIRST", "THIRD"]
. When "FIRST" is declared before "SECOND", this call returns ["second", "FIRST", "third", "SECOND", "THIRD"]
and the test fails.
I tracked it down to this line. If I change [identifier]
to [identifier.upcase]
, the test passes no matter what order the expressions are declared in the hash.
When I have this line changed to end with [identifier.upcase]
, DependencyResolver.find_resolve_order(dependencies)
returns ["THIRD", "SECOND", "FIRST"]
.
I found that if I change the calculator initialization to include case_sensitive: false
, then the test will also pass and the resolved dependencies are ["THIRD", "SECOND", "FIRST"]
.
I think the problem is that StringCasing
is used to downcase the hash keys/identifiers when case sensitive is false
, but the expressions are not downcased.