Added the chance to declare Functions with variable number of arguments

[MFrancesco]

Using this library we felt the need to add the chance to declare Functions with variable number arguments such as avg, min, max.

The feature has been tested in FunctionsWithVariableArgsTest and require few changes.

[fasseg]

Hola @MFrancesco
I will take a look at your implementation soon, but we had a earlier pull request addressing the same issue from @vavrecan:
#15
Can you please verify that the issues in the comments do not happen with your implementation?

[MFrancesco]

Ok, i'll write some tests to check that.

[MFrancesco]

Hi @fasseg,
i've added a new test to reproduce the bug in #15 .
The test

Expression e = new ExpressionBuilder("sum(1,avg(1.11,-3.14,2.03))")
.functions(variableArgsFunctions)
.build();
assertEquals(1.0d, e.evaluate(), 1.0d);

is passing.

[fasseg]

@MFrancesco:
Thanks for you input I will have a look today and merge if I don't find any issues.

[fasseg]

Hey @MFrancesco,
I noticed you introduced API changes by removing Function.getNumArguments() and adding Function.getMaxNumArguments() and Function.getMinNumArguments(). This might break existing user code.
Maybe we should introduce a new Subclass for var-arg functions, or even throw an exception when calling getNumArguments() for var arg functions, which should be safe from a User POV:
Something like this might be enough:

public int getNumArgument() {
    if (minArguments != maxArguments) {
        throw new UnsupportedOperationException("Calling getNumArgument() is not supported for var arg functions, please use getMaxNumArguments() or getMinNumArguments()");
    }
    return minArguments;
}

@MFrancesco, @sarxos, @leogtzr, @dktcoding: What do you guys think?

[MFrancesco]

It's ok for me. My mistake for not considering this aspect.
Il 31/lug/2015 09:59, "frank asseg" [email protected] ha scritto:

Hey @MFrancesco https://github.com/MFrancesco,
I noticed you introduced API changes by removing
Function.getNumArguments() and adding Function.getMaxNumArguments() and
Function.getMinNumArguments(). This might break existing user code.
Maybe we should introduce a new Subclass for var-arg functions, or even
throw an exception when calling getNumArguments() for var arg functions,
which should be safe from a User POV:
Something like this might be enough:

public int getNumArgument() {
if (minArguments < maxArguments) {
throw new UnsupportedOperationException("Calling getNumArgument() is not supported for var arg functions, please use getMaxNumArguments() or getMinNumArguments()");
}
return minArguments;
}

@MFrancesco https://github.com/MFrancesco, @sarxos
https://github.com/sarxos, @leogtzr https://github.com/leogtzr: What
do you guys think?

—
Reply to this email directly or view it on GitHub
#42 (comment).

[sarxos]

Hi @fasseg, I like this idea much better than doing refactor to call both getMin*() and getMax*() instead of getNumArgument().

[MFrancesco]

Hi @fasseg, just added the getNumArguments method in the last commit 51e4d54
Let me know if something else is needed.

[fasseg]

I scheduled some time next week to roll a new release and will probably merge this into master.

[boctulus]

Sorry, any example of how to implement custom functions like sum() prod() or avg() ? I see some code but it is for testing: https://github.com/fasseg/exp4j/blob/master/src/test/java/net/objecthunter/exp4j/ExpressionBuilderTest.java

[sarxos]

@boctulus,

Here is example of a very simple if function I have in my solution:

/**
 * Logical "if" function.
 */
public static final Function IF = new Function("if", 3) { // named "if", takes 3 arguments

	@Override
	public double apply(double... args) {
		validate(this, args);
		if (args[0] == 1) {
			return args[1];
		} else {
			return args[2];
		}
	}
};

Then I bind it with expression in this way:

String expression = "if (x > sin(2 * y), 1, 0)"; // sample expression
Expression exp = new ExpressionBuilder(expression)
	.variable("x")
	.variable("y")
	.operator(NOT, EQ, NEQ, GT, LT, GTEQ, LTEQ) // some operators I have defined
	.function(IF)
	.build();

The operator example:

/**
 * Greater than operator.
 */
protected static final Operator GT = new Operator(">", 2, true, Operator.PRECEDENCE_ADDITION - 1) {

	@Override
	public double apply(double... values) {
		validate(this, values);
		return values[0] > values[1] ? 1d : 0d;
	}
};

[boctulus]

Thank you very much @sarxos, I have a problem with variable number of arguments.... can you check this?

    public static final Function sum = new Function("sum") {
        @Override
        public double apply(double... args) {
            double sum = 0;

            for (int i=0; i<args.length; i++) {
                if ((double) args[i] != args[i]) {
                    throw new IllegalArgumentException(String.format("Argument %d has to be an integer",i));
                }
                if (args[i] < 0) {
                    throw new IllegalArgumentException(String.format("The argument %d can not be less than zero",i));
                }
            }

            if (args.length==1)
                for (int i= 0; i<=(int) args[0];i++) {
                    sum += i;
                }
            else
                if (args.length==2)
                    for (int i= (int) args[0]; i<=(int) args[1];i++) {
                        sum += i;
                    }
                 else
                    throw new IllegalArgumentException("Sum() requires one or two arguments!");

            return sum;
        }
    };

It works for one argument but If push two then I get this error:

java.lang.IllegalArgumentException: Invalid number of items on the output queue. Might be caused by an invalid number of arguments for a function.

Thanks again ...and happy new year!

[patrickguenther]

What is the current status for this feature? It would be super nice to have it. Can I try and take it up?

As far as I understand one of the issues is that this PR changes the public API. Possible solutions:

• allow -1 as a magic return value for getNumArguments() meaning "any number of arguments" and let the Function implementer deal with cases of not having been passed at least a minimum number of arguments. The exception however, can only be thrown at evaluation time, so this one is not so good.
• The solution suggested by @fasseg above. Personally I would not prefer throwing and handling an Exception if getNumArguments is being called with different minArgs and maxArgs, because of the overhead of the JVM having to create the stack trace. If we don't want to introduce a new VarArgsFunction base class, I would identify getNumArguments() as the minimum number of arguments and add a new Function getMaxNumArguments() for the maximum number of arguments, with an additional constructor for the field. The question is, what should identify the case for unlimited max arguments? Could be either Integer.MAX_VALUE, -1 or anything smaller than the regular numArguments. I would not pick 0 for unlimited arguments because of possible functions having 0...unlimited arguments.

Which one would be preferred?

[patrickguenther]

From inspecting the code, it seems like they went with solution 2. I would however like to see some more tests with more complicated nested expressions.

Other than that, the PR seems solid.

[fasseg]

Hey Patrick,

Sorry to inform you that exp4j is no longer actively developed. I don't have the time to maintain the project further and I have not yet found a dependable person to take over as a maintainer. :(