mbeckerle commented on code in PR #1426:
URL: https://github.com/apache/daffodil/pull/1426#discussion_r1949347092
##########
daffodil-lib/src/test/scala/org/apache/daffodil/lib/util/TestMaybeInlineForeach.scala:
##########
@@ -99,9 +99,9 @@ final class TestMaybeInlineForeach {
*/
// @Test
def testForeachVersusIfDefined(): Unit = {
- val foreachNanos: Double = time(testForeach)
- val ifDefinedNanos: Double = time(testIfDefined)
- val ifNullNanos: Double = time(testIfNull)
+ val foreachNanos: Double = time(testForeach()).toDouble
+ val ifDefinedNanos: Double = time(testIfDefined()).toDouble
+ val ifNullNanos: Double = time(testIfNull()).toDouble
Review Comment:
Looking at this from gitlab reviews, I have no clue what the rules are for
when the parens are required and when they're preferred present, preferred
absent, and when they are not allowed.
So I asked ChatGPT which gave this summary:
- Omit () for pure methods (no side effects).
- Use () for methods with side effects.
- Be consistent when overriding methods.
- Java methods require () when they are defined without parameters.
Of course we have lots of lazy vals in our scala code which both compute
things and have the side-effect of accumulating warnings/errors. So those will
break the rules here depending on how you interpret the first one.
I believe the right rule is that you omit () for methods with no
**_conceptual_** side-effects - none the caller is supposed to think about when
deciding when/where to call it.
Ex: If a function maintains a cache so that it can answer repeated calls
with the same arguments faster, the cache is invisible to the caller, so no ()
required on the call.
In C++ they used to call this "conceptual const-ness".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
