Normalising ugly method parameters using Guava

Question

I have a class which contains an optional Map:

 private Optional<ImmutableMap<String, String>> stuff;

In my class constructor I am passed Map<String, String> inputStuff, where inputStuff could be:

null
an empty Map
a populated Map

For the first two instances I need to store Optional.absent() and for the third instance I need to store an Optional immutable copy of the map. The best that I can come up with in terms of handling this is:

    final ImmutableMap<String, String> tmp = ImmutableMap.copyOf(Objects.firstNonNull(inputStuff, ImmutableMap.<String, String>of()));
    if (inputStuff.isEmpty())
    {
      this.stuff = Optional.absent();
    }
    else
    {
      this.stuff = Optional.of(inputStuff);
    }

Is there a cleaner way to handle this?

Is there a reason you're using `Optional` or `null` here at all, rather than just using the empty map to represent the empty map? — Louis Wasserman, Sep 14 '13 at 20:32
It's for consistency with other bits of the code, where there is a difference between an absent map and an empty map. — , Sep 14 '13 at 21:17

Grzegorz Rożniecki · Accepted Answer · 2013-09-14T10:23:29.053

8

Why not just simply do:

if (inputStuff == null || inputStuff.isEmpty()) {
  this.stuff = Optional.absent();
} else {
  this.stuff = Optional.of(ImmutableMap.copyOf(inputStuff));
}

I don't see a reason why should you create a temporary variable here. If you prefer using ternary operator, you can even avoid duplicated assigment to this.stuff.

edited Sep 14 '13 at 10:23

answered Sep 14 '13 at 10:18

Grzegorz Rożniecki

27,415
11
90
112

Yeah reason the code looks like it did is that I was hoping for some additional method that could be used to simply convert `tmp` to `absent`, somewhat similar to `Strings.emptyToNull()`, and so avoid the multi-line initialisation. Given that there isn't anything like that out there I'll mark this answer as correct. Thanks for the response! – Sep 14 '13 at 21:20
2

[`Iterables.isNullOrEmpty`](http://code.google.com/p/guava-libraries/wiki/IdeaGraveyard#Iterables.isNullOrEmpty) was explicitely rejected by Guava team, see [this answer](http://stackoverflow.com/questions/6910002/google-guava-isnullorempty-for-collections/6921270#6921270). – Grzegorz Rożniecki Sep 14 '13 at 21:25

Fabian Barney · Answer 2 · 2013-09-14T10:32:53.630

1

I would go with this:

this.stuff = (inputStuff == null || inputStuff.isEmpty()) 
   ? Optional.absent()
   : Optional.of(ImmutableMap.copyOf(inputStuff));

Or the way @Xaerxess posted it. It's much more straight-forward and easier to guess what's going on here.

edited Sep 14 '13 at 10:32

answered Sep 14 '13 at 10:27

Fabian Barney

14,219
5
40
60

Normalising ugly method parameters using Guava

2 Answers2