Null Pointer Exception despite Optional<>

Question

I have:

List<Optional<MyObject>> myList;

This list is populated by reading in from files. Once the file read is finished, I do a check against empty list: if(myList.size() == 0){//} and then proceed as follows:

myList.stream()
    .filter(Optional::isPresent)
    .map(i → i.orElse(new MyObject(“adventureBook”, 20))
    .collect(groupingBy(MyObject::getBookType, TreeMap::new, mapping(MyObject::getBookPrice, toList())));

I have around 350k MyObject files to read in from, 300k files are read in fine but when I try to read in the entire batch of c.350k files it throws a null pointer exception on the collect().

How is it possible that despite wrapping in Optional<> and checking Optional::isPresent, Optional::orElse etc still a null object manages to sneak through and given that I have such a large number of files what is the best way to try and narrow down on the errant file(s)? Thanks

edit: stack trace added

Exception in thread "main" java.lang.NullPointerException
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    at com.mypackage.MyObject.main(MyObject.java:108)

Having a list of optionals is already an odd thing, the non-existent objects shouldn't have ended up in the list in the first place. — Mark Rotteveel
– Mark Rotteveel, Commented Aug 25, 2018 at 14:02
and even if there could be one MyObject that is null, collecting those 20(s) to a List is well rather an interesting choice :) what if there is a real book (adventureBook ) with price 20? How will u make a difference between the two? — Eugene
– Eugene, Commented Aug 25, 2018 at 14:05
And Optional doesn't magically protect you against NullPointerException if you - for example - add null to that list instead of Optional.empty(). — Mark Rotteveel
– Mark Rotteveel, Commented Aug 25, 2018 at 14:05

Eugene · Accepted Answer · 2018-08-25 14:02:51Z

2

Well may be one of the properties in the non-null MyObject is null? You are indeed doing filter(Optional::isPresent), but that does not mean that the fields themselves are not null. MyObject::getBookType or MyObject::getBookPrice can easily be still null.

edited Aug 25, 2018 at 14:02

answered Aug 25, 2018 at 13:57

Eugene

122k17 gold badges219 silver badges335 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

shanlodh Over a year ago

Thanks to all who’ve responded. Taking the comments into consideration, I put in further null check before the file read arguments were passed to the MyObject constructor but problem still persists, the stack trace is too long for comments, I'm editing it to the OP.

Eugene Over a year ago

@shanlodh what is on line 108 from MyObject?

shanlodh Over a year ago

it's the .collect() mentioned in the OP

shanlodh Over a year ago

the 350k files I'd mentioned are distributed across c.15k folders and it appears that there were some non .gz2 files in a few folders that were upsetting the org.apache.commons.compress.compressors.* deserializers I was using. Using a helper class implementing FileNameFilter solved the issue. Thanks all once again

Eugene Over a year ago

@shanlodh you do understand that this is literally unrelated with the question itself, right? good think you solved it, my next time provide all the details needed

|

user3151902 · Accepted Answer · 2018-09-24 10:43:16Z

-1

Regarding the question "what is the best way to try and narrow down on the errant file(s)?", I usually use a pattern like this when parsing:

for (File file : filesToParse) {
    try {
        parseFile(file);
    } catch (IOException e) {
        // rethrow the exception, and make sure that
        // 1. you give the file in the message, to help narrow the error down
        // 2. pass the original exception as 2nd parameter, to preserve the stack trace
        throw new IOException("Failed to parse file: " + file, e);
    }
}

edited Sep 24, 2018 at 10:43

answered Aug 25, 2018 at 14:06

user3151902

3,1881 gold badge22 silver badges36 bronze badges

6 Comments

Zabuzard Over a year ago

You should not catch all runtime exceptions, only the one you are interested in, like IOException. Catching everything should only be done as super wrapper around your main app, and only for logging.

user3151902 Over a year ago

My understanding is that specific runtime exceptions should not be caught anyways, so I don't see a problem in throwing a different runtime exception, with more details to help debugging. See stackoverflow.com/questions/45607947 for example. IOException is not a runtime exception, so would need to be handled differently, if thrown.

user3151902 Over a year ago

@Zabuza What would be your suggested solution?

Zabuzard Over a year ago

Only catch IOException. Let the rest through. And if you truly want to catch everything, you need to include the errors too. So Throwable instead of RuntimeException. Though its questionable whether one should piggyback a RuntimeException such as IllegalStateException on an error.

user3151902 Over a year ago

@Zabuza Like this?

|

Collectives™ on Stack Overflow

Null Pointer Exception despite Optional<>

2 Answers 2

6 Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related