0

I've always assumed every time I call a method in Java the method is executed again. I've assumed the return value is not stored automatically unless I store it in a variable.

Then I ran across this code in Princeton's algs4.BST class where they call three methods twice each:

private boolean check() {
    if (!isBST())            StdOut.println("Not in symmetric order");
    if (!isSizeConsistent()) StdOut.println("Subtree counts not consistent");
    if (!isRankConsistent()) StdOut.println("Ranks not consistent");
    return isBST() && isSizeConsistent() && isRankConsistent();
}

Are they simply not concerned with performance? Or is the compiler smart enough keep the first return value of each method to use in the return statement?

Sorry if this is a duplicate seems like this answer should exist but I can't find it here or in Java docs. I found these (and others) but they don't answer my question:

Is this the cleanest way to repeat method call in Java?

How to call a method without repeat from other methods in java

JimLohse
  • 1,209
  • 4
  • 19
  • 44
  • 2
    They are not concerned with performance. Whoever wrote that code may also have a functional programming bent, so that they find storing results in variables distasteful. That doesn't change the fact that calling the same method again will incur its cost again, and that if nothing has changed then the same result will be produced. – John Bollinger Dec 05 '17 at 22:12
  • Haha if they are functional programmers like the ones I know they wouldn't touch Java – JimLohse Dec 05 '17 at 22:14
  • I've tried searches such as `oracle java docs repeat method call recalc`, perhaps @JohnBollinger answer is so obvious it's not documented? – JimLohse Dec 05 '17 at 22:19
  • The ultimate authority is the Java Language Specification. For Java 8, the relevant section is [JLS8 15.12.4](https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.4). Note well that it positively and unconditionally specifies that evaluating a method invocation expression involves "control is transferred to the method code". – John Bollinger Dec 05 '17 at 22:34
  • @JohnBollinger Ah thanks for your far superior knowledge of those docs, if you want to post as answer I'll accept, thanks again – JimLohse Dec 05 '17 at 22:37
  • @JohnBollinger: I think you are being slightly unfair to the authors. – rici Dec 06 '17 at 01:05

2 Answers2

1

The Java Language Specification explicitly and unconditionally states that evaluation of a method invocation expression involves executing the designated method's code. JLS 8 puts it this way:

At run time, method invocation requires five steps. First, a target reference may be computed. Second, the argument expressions are evaluated. Third, the accessibility of the method to be invoked is checked. Fourth, the actual code for the method to be executed is located. Fifth, a new activation frame is created, synchronization is performed if necessary, and control is transferred to the method code.

(JLS 8, 15.12.4; emphasis added)

Thus, invoking a method a second time incurs its cost a second time, regardless of whether the same value can be expected to be computed. It is conceivable that JIT compilation could optimize that, but there are more considerations there than whether the same result is going to be computed, and you're anyway unlikely to see any JIT action triggered by just two invocations of a given method.

Bottom line: yes, the author of the code was simply not concerned with performance. They may have considered the implementation presented to be clearer than one that avoided redundant method invocations, or they may have had some other personal reason for the choice. This kind of disregard for practicalities is not uncommon in code serving academic purposes, such as that presented.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • "This kind of disregard for practicalities" yeah wait til the professors start paying their own AWS bills, thanks for great answer! – JimLohse Dec 05 '17 at 22:53
1

The check function in that code is only called in the context:

assert check();

Asserts are not normally enabled in production code, and if asserts are not enabled, the assert statement does absolutely nothing. So the check function will only be run in debugging runs. That fact does not give license to be arbitrarily inefficient, but it is common to make no attempt to optimize such code. The point of code run as an assertion is to be obviously and indisputably correct while it verifies an invariant, precondition or postcondition, and optimizations -- even trivial ones like saving a result in a local variable -- do not contribute to that goal.

##14.10. The assert Statement

An assertion is an assert statement containing a boolean expression. An assertion is either enabled or disabled. If an assertion is enabled, execution of the assertion causes evaluation of the boolean expression and an error is reported if the expression evaluates to false. If the assertion is disabled, execution of the assertion has no effect whatsoever.

Community
  • 1
  • 1
rici
  • 234,347
  • 28
  • 237
  • 341
  • Good point, good answer, in the narrow context where this is only called by an assert, you make a great point and I missed that context when I saw this code. At the same time, I was asking on a different level and the accepted answer answered a wider question. Thanks for your answer, it fills in the details of the implementation in the case of the algs4 BST.java – JimLohse Dec 06 '17 at 06:06
  • 1
    I do not agree. Assertions are not disabled “in production code”, but turned on or off by a *startup option*, which is available whether in production environment or not. There is a recommendation to *always* enable assertions, unless there’s truly a performance issue with them.. So making the test code inefficient might be the actual cause for disabling the assertion in a production run, making the excuse circular… – Holger Dec 06 '17 at 11:36
  • @holger That is certainly a respectable position, but it is far from universal. Another opinion is that assertions should always be expensive because cheap checks should never be disabled. Discouraging expensive assertions also discourages using them to precisely document invariants and postconditions, for example. The checks in question -- that is, in this question -- are already expensive, even if only executed once, since they require a complete tree walk. Enabling them would make log-linear algorithms quadratic. Otoh, the checks serve an obvious useful purpose while debugging... – rici Dec 06 '17 at 16:55
  • ... and it is easy to see why they would be included in academic software. I fear that a discussion of this point will fall into the "primarily opinion-based" category but I will attempt to edit my answer in order to at least expose the underlying stylistic considerations. – rici Dec 06 '17 at 16:57
  • I see your point. But if the checks are *that* expensive, there’s really no excuse to invoke these methods twice. This problem wouldn’t exist if this method simply threw appropriate `AssertionError`s with detail message instead of combining poor man’s logging (printing) with returning a sole `boolean` yielding an `AssertionError` without a specific message… – Holger Dec 07 '17 at 07:39