[
https://issues.apache.org/jira/browse/GROOVY-12032?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18082973#comment-18082973
]
ASF GitHub Bot commented on GROOVY-12032:
-----------------------------------------
Copilot commented on code in PR #2552:
URL: https://github.com/apache/groovy/pull/2552#discussion_r3291074293
##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -2022,6 +2022,56 @@ public void remove() {
}
}
+
//--------------------------------------------------------------------------
+ // clamp
+
+ /**
+ * Constrains a Comparable value to lie within the inclusive range
+ * <code>[lower, upper]</code>. Returns <code>lower</code> if
<code>self</code>
+ * is less than <code>lower</code>, <code>upper</code> if
<code>self</code> is
+ * greater than <code>upper</code>, otherwise <code>self</code>.
+ * <pre class="language-groovy groovyTestCase">
+ * assert 10.clamp(1, 15) == 10
+ * assert 10.clamp(1, 5) == 5
+ * assert 10.clamp(12, 20) == 12
+ * assert 'a'.clamp('b', 'z') == 'b'
+ * </pre>
+ *
+ * @param self a Comparable value
+ * @param lower the inclusive lower bound
+ * @param upper the inclusive upper bound
+ * @return the clamped value
+ * @throws IllegalArgumentException if <code>lower</code> is greater than
<code>upper</code>
+ * @since 6.0.0
+ */
+ public static <T extends Comparable<? super T>> T clamp(T self, T lower, T
upper) {
+ if (ScriptBytecodeAdapter.compareGreaterThan(lower, upper)) {
+ throw new IllegalArgumentException("lower bound (" + lower + ")
must not be greater than upper bound (" + upper + ")");
+ }
+ if (ScriptBytecodeAdapter.compareLessThan(self, lower)) return lower;
+ if (ScriptBytecodeAdapter.compareGreaterThan(self, upper)) return
upper;
+ return self;
+ }
+
+ /**
+ * Constrains a Comparable value to lie within the given Range.
+ * <pre class="language-groovy groovyTestCase">
+ * assert 10.clamp(12..20) == 12
+ * assert 10.clamp(1..5) == 5
+ * </pre>
Review Comment:
The inline `groovyTestCase` examples don't currently cover reverse ranges or
exclusive-right ranges; with the current implementation, reverse ranges would
fail (and even after fixing, adding assertions helps prevent regressions).
Consider adding assertions like `10.clamp(20..12) == 12` and `10.clamp(1..<5)
== 4`.
##########
src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java:
##########
@@ -2022,6 +2022,56 @@ public void remove() {
}
}
+
//--------------------------------------------------------------------------
+ // clamp
+
+ /**
+ * Constrains a Comparable value to lie within the inclusive range
+ * <code>[lower, upper]</code>. Returns <code>lower</code> if
<code>self</code>
+ * is less than <code>lower</code>, <code>upper</code> if
<code>self</code> is
+ * greater than <code>upper</code>, otherwise <code>self</code>.
+ * <pre class="language-groovy groovyTestCase">
+ * assert 10.clamp(1, 15) == 10
+ * assert 10.clamp(1, 5) == 5
+ * assert 10.clamp(12, 20) == 12
+ * assert 'a'.clamp('b', 'z') == 'b'
+ * </pre>
+ *
+ * @param self a Comparable value
+ * @param lower the inclusive lower bound
+ * @param upper the inclusive upper bound
+ * @return the clamped value
+ * @throws IllegalArgumentException if <code>lower</code> is greater than
<code>upper</code>
+ * @since 6.0.0
+ */
+ public static <T extends Comparable<? super T>> T clamp(T self, T lower, T
upper) {
+ if (ScriptBytecodeAdapter.compareGreaterThan(lower, upper)) {
+ throw new IllegalArgumentException("lower bound (" + lower + ")
must not be greater than upper bound (" + upper + ")");
+ }
+ if (ScriptBytecodeAdapter.compareLessThan(self, lower)) return lower;
+ if (ScriptBytecodeAdapter.compareGreaterThan(self, upper)) return
upper;
+ return self;
+ }
+
+ /**
+ * Constrains a Comparable value to lie within the given Range.
+ * <pre class="language-groovy groovyTestCase">
+ * assert 10.clamp(12..20) == 12
+ * assert 10.clamp(1..5) == 5
+ * </pre>
+ *
+ * @param self a Comparable value
+ * @param range a Range defining the inclusive bounds
+ * @return the clamped value
+ * @since 6.0.0
+ */
+ @SuppressWarnings("unchecked")
+ public static <T extends Comparable<? super T>> T clamp(T self, Range<T>
range) {
+ T lower = range.isReverse() ? (T) range.getTo() : (T) range.getFrom();
+ T upper = range.isReverse() ? (T) range.getFrom() : (T) range.getTo();
Review Comment:
`Range#getFrom()`/`getTo()` are documented as the lower/upper bounds (and
`ObjectRange` normalizes so `from <= to` even when `isReverse()` is true).
Swapping bounds based on `range.isReverse()` inverts them for reverse ranges
(e.g. `5..1` or `'z'..'a'`), causing `clamp(self, lower, upper)` to throw
`IllegalArgumentException` even though the range is valid. Use
`range.getFrom()` as the lower bound and `range.getTo()` as the upper bound
regardless of `isReverse()`, and this should also let you drop the unchecked
casts and `@SuppressWarnings`.
> Provide a DGM clamp method
> --------------------------
>
> Key: GROOVY-12032
> URL: https://issues.apache.org/jira/browse/GROOVY-12032
> Project: Groovy
> Issue Type: New Feature
> Reporter: Paul King
> Priority: Major
>
> Similar to the one in:
> https://github.com/timyates/groovy-common-extensions
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
