metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Casey Stella <ceste...@gmail.com>
Subject Re: METRON-378 - variables that don't exist with STELLAR built in arithmetic functions
Date Mon, 31 Oct 2016 15:16:32 GMT
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 <ottobackwards@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?
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message