new math helper method

[sclatter]

Please accept my new math helper method. :)

-Sarah

[vybs]

2 comments

the expressions should support dynamic args such as the $idx in the loop or even the value in the context/ JSON ( so we should use the tap method )

I really dont see a use case yet where this comes handy,

can you tell me about your use case where this helper comes handy

[sclatter]

This originally came out of wanting to do modulus on idx and otherwise. We can down-scope this to have specific helpers - modulus, ceil, floor, random, etc. rather than a general method.

Jimmy had a suggestion of using nth-child selectors for modulus, but I feel that is mixing metaphors. I would prefer to have a specific modulus helper if the math/eval helper is deemed a bad idea.

[vybs]

thanks @sclatter , we shall resolve this tomm meeting for sure.

[sclatter]

Per the email thread, changed the generic math helper to a mod (modulus) helper and updated the unit tests. Please review and let me know if anything else is required.

[jimmyhchan]

How do you feel about leveraging the filter function used by @select instead? see jimmyhchan@09cacbc

I added the automatic lookup of $idx in filter but it feels a little weird.

[sclatter]

Jimmy,

I like leveraging the filter and defaulting to $idx. I would prefer to use "dividend" instead of "key" to stick with the proper mathematical term, since we are billing these as math helpers.

Sarah

[vybs]

• 1 for key infact, since @ eq , @ lt etc all use key including select

[jimmyhchan]

One option is to add another layer to the lookup:

• (add this) use "dividend" if it's defined in the parameter list for @mod
• else use "key" if defined in the parameter list for @mod
• else use "key" defined in the parameter list for @select if we are inside a select helper.
• else use $idx defined in the current context.

Reusing the filter function led to the awkwardness of using "key". Plus unlike\@eq @lt etc. @mod (and most math functions) needs 3 parameters e.g a mod b === c.

[sclatter]

I like this thinking. Though, I'm not sure if key defined in select is overkill. I might recommend the following logic:

(add this) use "dividend" if it's defined in the parameter list for @mod
else use "key" if defined in the parameter list for @mod
else use $idx defined in the current context.

[rragan]

{@math key="a" mod="b" eq="c"/} returns true/false vs {@math key="a" mod="b"/} returns value

Extendable:
{@math key="a" ceil="b"/}, {@math key="a" floor="b" /} and could have optional 3rd arg for testing

[vybs]

@jimmyhchan I dont like so many variations, just makes its much harder and leads to more bugs

Lets keep it simple then, use dividend, dont tie this to select

why cant we make idx explicit ? assuming too many things inc ode mean more to learn for a dev

[vybs]

• 1 for @rragan

[sclatter]

I like @rragan 's suggestion. Let's go with that - more straightforward and extensible. Me or Jimmy to code? I have some time this aft and Monday.

[jimmyhchan]

+1 @rragan. With this approach, +1 for keeping $idx explicit and not tying it to @select or the filter function.

@sclatter -- would you mind continuing this please... i can help but it won't have time unless its over the weekend.

[sclatter]

@jimmyhchan - yes, I'm on it! Thanks.

[travisbot]

This pull request passes (merged 1209b40 into eb0c016).

[sclatter]

Hi all,

I have committed my latest math helper. Please review. If it is to your liking, I can work up the documentation today.

Thanks,
-Sarah

[jimmyhchan]

Now that we are explicitly defining the method, can we get rid of this eval?

[travisbot]

This pull request passes (merged 329537d into eb0c016).

[sclatter]

Removed eval. Thanks, Jimmy! I feel we should hold off on the body thing as to keep it simple for now. That can be added later without disrupting backward compatibility.

[jimmyhchan]

merge it!

agreed. awesome work btw.
@math method="add" will also address Issue #80: Need $counter or a startsIndex parameter for loops

[travisbot]

This pull request passes (merged 5f472bc into eb0c016).

[travisbot]

This pull request passes (merged 6995032 into eb0c016).