Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce random JsonPath function #4

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Conversation

uarlouski
Copy link
Member

No description provided.

@uarlouski uarlouski requested a review from valfirst March 1, 2024 12:07
README.md Outdated
@@ -73,6 +73,9 @@ The function output is dictated by the function itself.
| `last()` | Provides the last item of an array | Depends on the array |
| `index(X)` | Provides the item of an array of index: X, if the X is negative, take from backwards | Depends on the array |
| `distinct()`| Provides an array containing only unique items from the input array | List<E> |
| `random()` | Provides the random item of an array | Depends on the array |

>>>>>>> Stashed changes
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>>>>>> Stashed changes

README.md Outdated
@@ -73,6 +73,9 @@ The function output is dictated by the function itself.
| `last()` | Provides the last item of an array | Depends on the array |
| `index(X)` | Provides the item of an array of index: X, if the X is negative, take from backwards | Depends on the array |
| `distinct()`| Provides an array containing only unique items from the input array | List<E> |
| `random()` | Provides the random item of an array | Depends on the array |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `random()` | Provides the random item of an array | Depends on the array |
| `random()` | Provides one random item from the array | Depends on the array |

.collect(Collectors.toList());

if (elements.isEmpty()) {
throw new JsonPathException("Aggregation function attempted random value on empty array");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new JsonPathException("Aggregation function attempted random value on empty array");
throw new JsonPathException("Aggregation function attempted to get random item from empty array");

throw new JsonPathException("Aggregation function attempted random value on empty array");
}

java.util.Random rand = new java.util.Random();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java.security.SecureRandom

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason to change it in this particular context? we do not care about security/predictability and just Random works much faster

java.util.Random rand = new java.util.Random();
return elements.get(rand.nextInt(elements.size()));
}
throw new JsonPathException("Aggregation function attempted random value using non array");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new JsonPathException("Aggregation function attempted random value using non array");
throw new JsonPathException("Aggregation function attempted to get random item from not array");

@uarlouski uarlouski force-pushed the feature/add-random-feature branch 2 times, most recently from 6f8dc4a to 500eaf9 Compare March 1, 2024 15:26
@valfirst valfirst merged commit eec830c into master Mar 1, 2024
9 checks passed
@valfirst valfirst deleted the feature/add-random-feature branch March 1, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants