JsonLogic.Net icon indicating copy to clipboard operation
JsonLogic.Net copied to clipboard

Support for lexicographical comparison for numeric operators

Open sarensw opened this issue 6 years ago • 3 comments

I want to use the < and > operators to compare strings lexicographically (Dates in form of yyyy-MM-dd to be more precisely). This works in JS but unfortunately not in this .Net port. It throws a FormatException because the strings cannot be converted to double.

I forked and tried to fix this but I'm having a hard time doing so due to the Func based nature in the code.

As a first try, I have changed DoubleArgsSatisfy so that it works in a generic way. But I'm still only able to set one type when calling this method in AddOperator

AddOperator(">", ArgsSatisfy<string>((prev, next) => string.CompareOrdinal(prev, next) > 0));

private Func<IProcessJsonLogic, JToken[], object, object> ArgsSatisfy<T>(Func<T, T, bool> criteria)
{
    return (p, args, data) => {
        var valuesString = args.Select(a => a == null ? "" : p.Apply(a, data).ToString()).ToArray();
        var values = valuesString.Select(v => (T) Convert.ChangeType(v, typeof(T))).ToArray();
        for (int i = 1; i < values.Length; i++) {
            if (!criteria(values[i-1], values[i])) return false;
        }
        return true;
    };
}

Can you give me a hint on how to improve this?

sarensw avatar Mar 11 '20 16:03 sarensw

I have created a pull request with a proposed solution for this. @yavuztor could you please have a look at whether this one is ok for you?

sarensw avatar Mar 13 '20 03:03 sarensw

I have created a pull request with a proposed solution for this. @yavuztor could you please have a look at whether this one is ok for you? @sarensw , the fix in the pull request is good, but could be easier to read with some refactoring. Can you check my comments to see if they make sense?

Also, I pushed some changes to work around the missing signing key issue on build. It should build fine next time.

yavuztor avatar Mar 21 '20 02:03 yavuztor

Hi, I have created a refactored pull request fixing this issue (#26). It is based on @sarensw's implementation.

@yavuztor, could you have a look at it?

FrederikLynggaard avatar Jul 03 '21 14:07 FrederikLynggaard