13

I have a piece of code where an interface has an Optional return method and some of the classes that implement it to return something, other don't.

In an effort to embrace this brilliant "null killer", here is what I have tried:

public interface Gun {
    public Optional<Bullet> shoot();
}

public class Pistol implements Gun{
    @Override
    public Optional<Bullet> shoot(){
        return Optional.of(this.magazine.remove(0)); 
    }//never mind the check of magazine content
}

public class Bow implements Gun{
    @Override
    public Optional<Bullet> shoot(){
        quill--;
        return Optional.empty();
    }
}

public class BallisticGelPuddy{
    private Gun[] guns = new Gun[]{new Pistol(),new Bow()};
    private List<Bullet> bullets = new ArrayList<>();
    public void collectBullets(){
        //here is the problem
        for(Gun gun : guns)
            gun.shoot.ifPresent(bullets.add( <the return I got with the method>)
}}

I apologise for how silly this example is.
How can I check the return I just got and add it only if present, using optional?

P.S. is there any real usefulness to Optional which is if(X != null) couldn't do?

Bohemian
  • 412,405
  • 93
  • 575
  • 722
Vale
  • 1,104
  • 1
  • 10
  • 29
  • 2
    "P.S. is there any real usefulness to Optional which is if(X != null) couldn't do?" - I'm a huge fan of Optional, but at the same time, I regard it as almost a form of comment. When you see something of type Optional, you should automatically *know* that it might not be there; however, most values that *can* hold null never *should*, so it's not general practice to surround absolutely every de-reference with null checks. – Edward Peters Jun 15 '16 at 22:31
  • @EdwardPeters I don't want to turn this in some kind of bloodbath, but I agree with [this guy here](http://huguesjohnson.com/programming/java/java8optional.html): what's the point? I agree on the impression "Oh, a optional! Better be sure that indeed it exists", but I think it could have been done a little better. – Vale Jun 15 '16 at 22:40
  • 3
    So, part of the benefit I see is clarity. Any time you see `Optional` getting assigned a `null` value, you *know* that it's wrong, and may feel justified in fixing it. When you see null getting assigned to another value... well, who knows, maybe that's appropriate. I also feel that, philosophically, "Thing which might be nothing" really should be a different type than "Thing that must be something." It could be done better, but only if it were part of the language from the ground up... and, IMO, Java is about one part pig to three parts lipstick. – Edward Peters Jun 15 '16 at 22:56

4 Answers4

23

I see where you're going with this - when a projectile (may be a better class name than Bullet) goes through BallisticGelPuddy, it either becomes stuck or it doesn't. If it gets stuck, it accumulates in BallisticGelPuddy.

Let's rewrite the code if we were using null checks instead:

for(Gun gun: guns) {
    final Bullet bullet = gun.shoot();
    if(bullet != null) {
        bullets.add(bullet);
    }
}

Pretty straightforward, right? If it exists we want to add it in.

Let's add the optional style back in:

for(Gun gun: guns) {
    gun.shoot().ifPresent(bullets::add);
}

Effectively these two things accomplish the same thing, although the Optional approach is terser.

In this scenario, there's really no difference between the two approaches since you're always going to be checking for existence. Optional is meant to guard against mistakes when handling null and allows you to express a more fluid call chain, but consider the practicality of using Optional in this scenario. It doesn't seem entirely necessary for this case.

Community
  • 1
  • 1
Makoto
  • 104,088
  • 27
  • 192
  • 230
13

I think you want:

gun.shoot().ifPresent(bullets::add);

Or you can dispense with the (coded) loop too:

guns.stream()
  .map(Gun::shoot)
  .filter(Optional::isPresent)
  .map(Optional::get)
  .forEach(bullets::add);

But it's uglier.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
3

With the stream API, you can do:

    List<Bullet> bullets = Arrays.stream(guns)
            .map(Gun::shoot)
            .flatMap(this::streamopt) // make Stream from Optional!
            .collect(Collectors.toList());

Unfortunately, in Java 8, there is no method which converts Optionals to Stream, so you need to write it yourself. See Using Java 8's Optional with Stream::flatMap

Community
  • 1
  • 1
user140547
  • 7,750
  • 3
  • 28
  • 80
0

I want to post this for future reference, for anybody tripping in a problem similar to mine. Should you want access to methods of what you have just returned, if present:

public class Bullet{
    private int weight = 5;
    public int getWeight(){ return weigth;}
}
public interface Gun {
    public Optional<Bullet> shoot();
}

public class Pistol implements Gun{
    @Override
    public Optional<Bullet> shoot(){
        return Optional.of(this.magazine.remove(0)); 
    }//never mind the check of magazine content
}

public class Bow implements Gun{
    @Override
    public Optional<Bullet> shoot(){
        quill--;
        return Optional.empty();
    }
}

public class BallisticGelPuddy{
    private Gun[] guns = new Gun[]{new Pistol(),new Bow()};
    private List<Bullet> bullets = new ArrayList<>();
    private int totWeigth = 0;
    public void collectBullets(){
        // IF YOU WANT TO ONLY ADD WHAT YOU HAVE FOUND IN A COMPATIBLE CLASS
        // thanks to makoto and bohemian for the answers
        for(Gun gun : guns)
            gun.shoot.ifPresent(bullets::add)
        //IF YOU WANT TO ACCESS THE RETURNED OBJECT AND ADD IT TOO
        for(Gun gun : guns)
            gun.shoot.ifPresent( arg -> {totWeight += arg.getWeigth();
                                         bullets.add(arg);});
}}
Vale
  • 1,104
  • 1
  • 10
  • 29