metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Otto Fowler <ottobackwa...@gmail.com>
Subject Re: METRON-378 - variables that don't exist with STELLAR built in arithmetic functions
Date Mon, 31 Oct 2016 15:48:59 GMT
Wouldn’t it be more correct to return nothing?


On October 31, 2016 at 11:44:48, Nick Allen (nick@nickallen.org) wrote:

>
> 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 <cestella@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 <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?
> >
>



-- 
Nick Allen <nick@nickallen.org>

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