Skript
Skript copied to clipboard
Add a getter method for converting in Expression class
Description
Similar to the Expression#check method, this method will get all the values and then use the provided converter to convert the values to the requested output. This is super handy for everyone, PropertyExpression currently has an almost exact method, so why not on Expression too to make things so much easier.
Example of how this method can be utilized:
- https://github.com/TheLimeGlass/Skript/blob/item-amount-type/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java
- https://github.com/SkriptLang/Skript/pull/4632#discussion_r928048655
This link above is a future pull request for when this gets merged to add support for itemtype checking in the item amount expression, related suggest #2142
The method should probably also be named getArray (or mb getConvertedArray) so reflect which method it uses internally.
The method should probably also be named
getArray(or mb getConvertedArray) so reflect which method it uses internally.
I was thinking about that, I was also thinking convert, but convert is already a method in SimplePropertyExpression and there is already Expression#getConverteredExpression I wanted to make it closer to the exact method it mimics which is PropertyExpression#get(Object[], Converter)
I think because Expression has multiple get methods (such as getArray and getAll, having different behaviour), it's better to be explicit in the naming which to use. Therefore, I think getArray (same name as regular method, just with a converter param) or getConvertedArray (still implying getArray, but also more explicitly indicating it has a converter) would be better
Needs reviews, this helps tremendously, makes code shorter and easier to read.
I'm not sure I see the value here. Why not expr.stream(e).map(converter::convert)?
I'm not sure I see the value here. Why not
expr.stream(e).map(converter::convert)?
Way shorter, direct usage to lambdas, and nested expressions. Streams you also have to convert to array, stream, and map.
Demonstrated difference in link mentioned https://github.com/SkriptLang/Skript/pull/4632#discussion_r928048655
I'm not sure I see the value here. Why not
expr.stream(e).map(converter::convert)?Way shorter, direct usage to lambdas, and nested expressions. Streams you also have to convert to array, stream, and map.
Demonstrated difference in link mentioned #4632 (comment)
Coping your example here for ease of viewing:

While I would agree that your second example looks cleaner, I feel like the use cases requiring the use of flatMap are somewhat rare. In addition, neither of those examples are ideal as they both call a getter method of the types expression multiple times.
This is still very much valid. Coming back because this still comes up with modern syntaxes.
TODO add stream collect method
@Override
protected PotionEffect[] get(Event event, LivingEntity[] source) {
return Arrays.stream(source)
.flatMap(entity -> entity.getActivePotionEffects().stream())
.toArray(PotionEffect[]::new);
}
vs a proper collect method
@Override
protected PotionEffect[] get(Event event, LivingEntity[] source) {
return collect(source, entity -> entity.getActivePotionEffects().stream());
// and another method for collections.
return collect(source, entity -> entity.getActivePotionEffects());
}