> > looking for some sage feedback, hopefully from someone who knows > the reasoning behind 'return 0d if null ‘.
Doesn't this originate from the idea that a Stellar expression should not blow-up if a message is missing a field (aka unable to resolve a variable)? I remember this as something that I had to get my head around when I started playing with Stellar. On Mon, Oct 31, 2016 at 11:16 AM, Casey Stella <ceste...@gmail.com> wrote: > I really don't like equating missing variable to 0 as 0 has meaning that > does not imply it is missing. What I'd rather see is a return of > Double.NaN for all situations where arguments are not a number. Just my > $0.02. > > On Fri, Oct 28, 2016 at 10:07 AM, Otto Fowler <ottobackwa...@gmail.com> > wrote: > > > I was looking at METRON-378, and was wondering if we could discuss a > little > > bit. > > > > This issue, as stated is that if your stellar expression references > > variables that are not passed in at runtime, the call still succeeds and > > passes back a value. > > > > From the issue: > > > > @Test(expected = ParseException.class) > > public void testMissingVariables() { > > String query = "1 + cannot + add + missing + variables"; > > run(query, new HashMap<>()); > > } > > > > > > This works, because getDouble is implemented thusly: > > > > private Double getDouble(Token<?> token) { > > Number n = (Number) token.getValue(); > > if (n == null) { > > return 0d; > > } else { > > return n.doubleValue(); > > } > > } > > > > Is the STELLAR function suite, we have to do the work to handle missing > > parameters and variables, but here, for things handled in the > > StellarCompiler we do not. > > > > So the first question is if we want to change this behavior to be > > consistent with the rest of the library, that is to throw an exception > with > > invalid parameters. > > > > Changing the above function to return null will accomplish that, with the > > side effect of having to refactor the validation calls. As we have > > discussed before, having validation implemented by passing in a known > > failure case and expecting an error is not really ideal, since it muddies > > the purpose of what you are validating ( syntactical correctness of the > > expression vs logical runtime correctness ) among other things. With a > > change like this, validation will get an exception, not the return we > > currently expect in the tests etc. > > > > Maybe we don’t want to handle this in the Compiler at all? Maybe > > StellarParser’s parse() function should validate parameters? This > would, I > > think, require a refactoring or new version of validate so that running > > validate inside of parse is not expensive and uses the existing > constructs > > built in the call. This would give ‘global’ validation to parameters I > > believe, and allow for some cleanup in the existing functions, at the > cost > > of the a per function default handling. > > > > Anyways, looking for some sage feedback, hopefully from someone who > knows > > the reasoning behind 'return 0d if null ‘. Maybe this is a documentation > > issue, or can be treated as such? > > > -- Nick Allen <n...@nickallen.org>