11

I've met a such kind of code and comments in the java.util.ImmutableCollections class:

static final class List0<E> extends AbstractImmutableList<E> {
    ...
    @Override
    public E get(int index) {
        Objects.checkIndex(index, 0); // always throws IndexOutOfBoundsException
        return null;                  // but the compiler doesn't know this
    } 
    ...
}

Why not just throw new IndexOutOfBoundsException(...)? What's the reason?

Andremoniy
  • 34,031
  • 20
  • 135
  • 241

3 Answers3

7

Perhaps, it is just an avoiding of a code duplication, because otherwise it would be something like:

new IndexOutOfBoundsException(outOfBoundsMessage(..., ...)) 

but outOfBounds* methods are private, so by design somebody should call wrappers like return Preconditions.checkIndex(index, length, null)

Andremoniy
  • 34,031
  • 20
  • 135
  • 241
  • 7
    Yes, avoiding code duplication. There is an effort towards centralizing all of these index checking and exception message constructions. That does not only affect collection implementations and `Arrays` utility methods, but also NIO buffers, `CharSequence` implementations and so on. Lots of places, lots of code duplication. Note that this is a [new API method](https://docs.oracle.com/javase/9/docs/api/java/util/Objects.html#checkIndex-int-int-) that allows your application to participate in this central handling. Still, I would use `throw new AssertionError()` instead of `return null;` here… – Holger Jan 12 '18 at 16:17
  • O master @Holger, I appreciate your appreciation of my answer and yours additional comment. Yours authoritative opinion is also very needed in this question: https://stackoverflow.com/questions/48227496/reference-to-method-is-ambiguous-when-migrating-from-java8-to-java9 – Andremoniy Jan 13 '18 at 22:07
3

Unless I'm missing something obvious here, this is much simpler. It's the same thing as:

// this will not compile, *even* if test throws the Exception always
public String s() {
    test(); 
}

private void test() {
    throw new RuntimeException("just because");
}

The compiler can't tell that test will always throw the RuntimeException so it requires a return statement in s(). Same thing happens in switch statements for an enum, where you have to provide a throw; even if you have handled all the cases of that enum.


This code btw is used in List0, where it does not make sense to call get(x), because there are no elements in the List for sure.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • You can always write `public String s() { throw new RuntimeException("aaa"); }` what is 100% valid and compilable code – Andremoniy Jan 11 '18 at 20:09
2

The implementation seems more design-oriented than just functionality here.

The reasons for the same could primarily be that the Preconditions.checkIndex is marked as an @HotSpotIntrinsicCandidate which means a code performance improvement is sought while this method is used internally.

Also, can notice that all the immutable list - containing 0(empty),1, 2 or N elements, created using the factory method of makes use of the Objects.checkIndex(int index, int length) which eventually relies on the above method call to possibly make some internal optimisations.


Brief over HotSpotIntrinsicCandidate from the library :-

The @HotSpotIntrinsicCandidate annotation is specific to the HotSpot Virtual Machine. It indicates that an annotated method may be (but is not guaranteed to be) intrinsified by the HotSpot VM.

A method is intrinsified if the HotSpot VM replaces the annotated method with hand-written assembly and/or hand-written compiler IR -- a compiler intrinsic -- to improve performance.

The @HotSpotIntrinsicCandidate annotation is internal to the Java libraries and is therefore not supposed to have any relevance for application code.

Naman
  • 27,789
  • 26
  • 218
  • 353
  • if you mentioned `@HotSpotIntrinsicCandidate` you could have mentioned `@ForceInline` for `Objects.checkIndex`. Also, this is interesting as *how* exactly is a CPU going to optimize this call being intrinsic; I don't know that btw – Eugene Jan 12 '18 at 07:44
  • @Eugene `ForceInline` seems to be focussing on ignoring the metrics of annotated methods hence avoided bringing it into the discussion. Haven't digged into the implementation of the `HotSpotIntrinsicCandidate` myself yet as well. – Naman Jan 12 '18 at 08:19
  • 1
    there is not much to see under the implementation... :) it's just an annotation, you would have to dig the jvm source filed and see what is *particularly* done for that method... – Eugene Jan 12 '18 at 08:21