2

Is it possible to rewrite the following using Optional ? It seems to me when the situation requires to throw exception then using Optional may not be a good idea ?

Item item = service.get(id);

if (item == null) {
 throw new ItemNotFoundException();
}

item.setValue(false);
itemDao.update(item);

Attempt 1:

Optional.ofNullable(service.get(id))
  .ifPresent(item -> {
    item.setValue(false);
    itemDao.update(item);
  });   // cannot throw exception inside the consumer

Attempt 2:

Optional.ofNullable(service.get(id))
  .map(item -> {
    item.setValue(false);
    itemDao.update(item);
  })   
  .orElseThrow(new ItemNotFoundException()); //can throw exception here but the usage of map is not correct
once
  • 536
  • 7
  • 21
  • In the first place, `Optional` is supposed to be used as return type by the method. If possible, change `service.get` to return `Optional`. – Zabuzard Aug 19 '21 at 08:25
  • 4
    Don't try to turn every null check into something that uses `Optional`. If statements that check for null are not deprecated. – Sweeper Aug 19 '21 at 08:25
  • You have to write `orElseThrow(ItemNotFoundException::new)`. I.e. you create a supplier that can create the exception **on demand**. – Zabuzard Aug 19 '21 at 08:26
  • 1
    Ideally you let the method return `Optional` in the first place. If thats not feasible, I would stick with regular if-else. Unless you want to return an Optional yourself. – Zabuzard Aug 19 '21 at 08:27
  • Does this answer your question? [Java 8 optional: ifPresent return object orElseThrow exception](https://stackoverflow.com/questions/41485751/java-8-optional-ifpresent-return-object-orelsethrow-exception) – Murat Karagöz Aug 19 '21 at 08:28
  • @MuratKaragöz I don't think it answers the question. – Sweeper Aug 19 '21 at 08:29
  • Besides the answers, I agree with @Sweeper. A simple if-null check is a clear control flow and suits perfectly fine for this case. – Murat Karagöz Aug 19 '21 at 08:29
  • @Sweeper It may not directly, but it's exactly the same question with some discussions going on. – Murat Karagöz Aug 19 '21 at 08:30
  • Note that you can always call `ifPresent` and `orElseThrow` as separate statements. You don't have to always chain these methods. – Sweeper Aug 19 '21 at 08:33

2 Answers2

4

As mentioned in answers before - the service method should return an optional. If you absolutely want to use an Optional with the unchanged service method then do it as follows:

Item item = Optional.ofNullable(service.get(id)).orElseThrow(ItemNotFoundException::new);

item.setValue(false);
itemDao.update(item);
WhyNot
  • 41
  • 3
0

You should return the item on map.

Optional.ofNullable(service.get(id))
.map(item -> {
   item.setValue(false);
   itemDao.update(item);
   return item;
}).orElseThrow(ItemNotFoundException::new);
Preston
  • 180
  • 9