Improve ExprElement to use iterators (#5183)

* Improve ExprElement to use iterators

* Improve ExprElement to use iterators

* Use colons in pattern

* Add xth last element syntax

* Add optional to

* Add trailing new line

Author: Pikachu920

Diff for: src/main/java/ch/njol/skript/expressions/ExprElement.java

@@ -32,23 +32,29 @@
 import ch.njol.util.Kleenean;
 import ch.njol.util.StringUtils;
 import ch.njol.util.coll.CollectionUtils;
+import com.google.common.collect.Iterators;
 import org.bukkit.event.Event;
 import org.eclipse.jdt.annotation.Nullable;
 
 import java.lang.reflect.Array;
+import java.util.Iterator;
 
 @Name("Element of")
 @Description({"The first, last or a random element of a set, e.g. a list variable.",
 		"See also: <a href='#ExprRandom'>random</a>"})
 @Examples("give a random element out of {free items::*} to the player")
-@Since("2.0")
+@Since("2.0, INSERT VERSION (relative to last element)")
 public class ExprElement extends SimpleExpression<Object> {
 
 	static {
-		Skript.registerExpression(ExprElement.class, Object.class, ExpressionType.PROPERTY, "(-1¦[the] first|1¦[the] last|0¦[a] random|2¦%-number%(st|nd|rd|th)) element [out] of %objects%");
+		Skript.registerExpression(ExprElement.class, Object.class, ExpressionType.PROPERTY, "(0:[the] first|1:[the] last|2:[a] random|3:[the] %-number%(st|nd|rd|th)|4:[the] %-number%(st|nd|rd|th) [to] last) element [out] of %objects%");
 	}
 
-	private int element;
+	private enum ElementType {
+		FIRST, LAST, RANDOM, ORDINAL, TAIL_END_ORDINAL
+	}
+
+	private ElementType type;
 
 	private Expression<?> expr;
 
@@ -58,34 +64,56 @@ public class ExprElement extends SimpleExpression<Object> {
 	@Override
 	@SuppressWarnings("unchecked")
 	public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
-		expr = LiteralUtils.defendExpression(exprs[1]);
-		number = (Expression<Number>) exprs[0];
-		element = parseResult.mark;
+		expr = LiteralUtils.defendExpression(exprs[2]);
+		type = ElementType.values()[parseResult.mark];
+		number = (Expression<Number>) (type == ElementType.ORDINAL ? exprs[0]: exprs[1]);
 		return LiteralUtils.canInitSafely(expr);
 	}
 
 	@Override
 	@Nullable
-	protected Object[] get(Event e) {
-		Object[] os = expr.getArray(e);
-		if (os.length == 0)
+	protected Object[] get(Event event) {
+		Iterator<?> iter = expr.iterator(event);
+		if (iter == null || !iter.hasNext())
 			return null;
-		Object o;
-		if (element == -1) {
-			o = os[0];
-		} else if (element == 1) {
-			o = os[os.length - 1];
-		} else if (element == 2) {
-			Number number = this.number.getSingle(e);
-			if (number == null || number.intValue() - 1 >= os.length || number.intValue() - 1 < 0)
-				return null;
-			o = os[number.intValue() - 1];
-		} else {
-			o = CollectionUtils.getRandom(os);
+		Object element = null;
+		switch (type) {
+			case FIRST:
+				element = iter.next();
+				break;
+			case LAST:
+				element = Iterators.getLast(iter);
+				break;
+			case ORDINAL:
+				assert this.number != null;
+				Number number = this.number.getSingle(event);
+				if (number == null)
+					return null;
+				try {
+					element = Iterators.get(iter, number.intValue() - 1);
+				} catch (IndexOutOfBoundsException exception) {
+					return null;
+				}
+				break;
+			case RANDOM:
+				Object[] allIterValues = Iterators.toArray(iter, Object.class);
+				element = CollectionUtils.getRandom(allIterValues);
+				break;
+			case TAIL_END_ORDINAL:
+				allIterValues = Iterators.toArray(iter, Object.class);
+				assert this.number != null;
+				number = this.number.getSingle(event);
+				if (number == null)
+					return null;
+				int ordinal = number.intValue();
+				if (ordinal <= 0 || ordinal > allIterValues.length)
+					return null;
+				element = allIterValues[allIterValues.length - ordinal];
+				break;
 		}
-		Object[] r = (Object[]) Array.newInstance(getReturnType(), 1);
-		r[0] = o;
-		return r;
+		Object[] elementArray = (Object[]) Array.newInstance(getReturnType(), 1);
+		elementArray[0] = element;
+		return elementArray;
 	}
 
 	@Override
@@ -97,7 +125,7 @@ public <R> Expression<? extends R> getConvertedExpression(Class<R>... to) {
 			return null;
 
 		ExprElement exprElement = new ExprElement();
-		exprElement.element = this.element;
+		exprElement.type = this.type;
 		exprElement.expr = convExpr;
 		exprElement.number = this.number;
 		return (Expression<? extends R>) exprElement;
@@ -116,17 +144,17 @@ public Class<?> getReturnType() {
 	@Override
 	public String toString(@Nullable Event e, boolean debug) {
 		String prefix;
-		switch (element) {
-			case -1:
+		switch (type) {
+			case FIRST:
 				prefix = "the first";
 				break;
-			case 1:
+			case LAST:
 				prefix = "the last";
 				break;
-			case 0:
+			case RANDOM:
 				prefix = "a random";
 				break;
-			case 2:
+			case ORDINAL:
 				assert number != null;
 				prefix = "the ";
 				// Proper ordinal number

Diff for: src/test/skript/tests/syntaxes/expressions/ExprElement.sk

@@ -0,0 +1,12 @@
+test "ExprElement":
+	set {_list::*} to "foo", "bar" and "foobar"
+	assert first element of {_list::*} is "foo" with "Incorrect first element"
+	assert last element of {_list::*} is "foobar" with "Incorrect last element"
+	assert 2nd element of {_list::*} is "bar" with "Incorrect 2nd element"
+	assert {_list::*} contains (random element of {_list::*}) with "Incorrect random element"
+	assert 2nd to last element of {_list::*} is "bar" with "Incorrect 2nd last element"
+	assert 3rd last element of {_list::*} is "foo" with "Incorrect 3rd last element"
+	assert 100th last element of {_list::*} is not set with "Incorrect 100th last element"
+	assert 1st last element of {_list::*} is "foobar" with "Incorrect 1st last element"
+	assert 0th last element of {_list::*} is not set with "Incorrect 0th last element"
+	assert -1th last element of {_list::*} is not set with "Incorrect -1th element"