Exception with Map.getOrDefault

[Piet Souris]

I have this code:

Map<String, Integer> map = new HashMap<>(Map.of("a", 0, "b", 0, "c", 0, "d", 0));
var s = "jnz c 2";
var array = s.split("\\s+");
var test = map.getOrDefault(array[1], Integer.parseInt(array[1]));  // error in this line

where the variable 'test' indicates whether a program counter must be adjusted by the last number. The command 's' can also come in this form: 'jnz 5 3'.

The last line gives me the exception:  Exception in thread "main" java.lang.NumberFormatException: For input string: "c".

Maybe I am getting too old for this, but what am I doing wrong?

[Tim Holloway]

Looks to me like you're trying to parse the string whose value is "c" as an integer. Definitely a NumberFormat problem, that.

You didn't forget that arrays start at 0, did you? When you split the string "jnz c 2"?

[Rob Spoor]

Don't you mean Integer.parseInt(array[2]) instead of Integer.parseInt(array[1])?

[Piet Souris]

It is day 12 of Advent Of Code 2016. This is the instruction:

jnz x y jumps to an instruction y away (positive means forward; negative means backward), but only if x is not zero.

Here x is either the variable a, b, c or d, or an int like 5.
As you see, I put the variables in a hashmap<String, Integer>, and the idea was that if array[1] (that is either that variable or an integer) is a key in the map, then take its value, or else array[1] is an integer that I will then parse.

It seems that java calculates the default value even if the key is present. But maybe there is some other explanation.

[Tim Holloway]

Yes, the compiler is compiling argument expressions for a method invocation. It has no way of knowing whether those arguments will be used or not.

[Paul Clapham]

You'd need a Supplier<Integer> to implement the idea that you'd only compute the value on demand, but Map.getOrDefault doesn't have a method Map.getOrDefault(Object, Supplier<V>).

[Mike Simmons]

Yeah.  You could achieve this with computeIfAbsent(), but it would have the side effect of storing the newly computed value in the map.  Which might be fine, but seems unnecessary for this use case.

    var test = map.computeIfAbsent(array[1], key -> Integer.parseInt(array[1]));

I think it's best to just code what you want directly yourself:

    var test = tryLookup(array[1], map);

  [...]

  private static int tryLookup(String s, Map<String, Integer> map) {
    var mapVal = map.get(s);
    return mapVal == null ? Integer.parseInt(s) : mapVal;
  }

[Tim Holloway]

Something more like

var test = map.contains(array[1]) ? map.get(array[1]) : Integer.parseInt(array[1]));

I think that the only the predicate that matches the test result gets computed.

[Mike Simmons]

Yes, that would work too.  (Yes, parseInt() is not called unless the contains() is false.)  I reflexively avoid the contains() method because it seems wasteful to do that and then a get() - I prefer to just do the get and test for null. But then I need another temp variable to hold it in, so I extracted a method, thinking Piet may use it again for other arguments in the code.  But, six of one, half dozen of the other - either way works.

[Tim Holloway]

Mike Simmons wrote:Yes, that would work too.  (Yes, parseInt() is not called unless the contains() is false.)  I reflexively avoid the contains() method because it seems wasteful to do that and then a get() - I prefer to just do the get and test for null. But then I need another temp variable to hold it in, so I extracted a method, thinking Piet may use it again for other arguments in the code.  But, six of one, half dozen of the other - either way works.

Æsthetically, I'd pre-calculate the key into a temporary variable, but that's largely for maintainability.  The compiler can optimize out any of the above.

To avoid contains/get, something like this:

var test = map.get(array[1]);
if ( test == null ) {
  test = Integer.parseInt(array[1]);
}

Not as elegant, but it avoid the extra lookup.

On the other hand, the trinary operation works when you want test to be final.

[Rob Spoor]

It can be made more elegant by using Optional:

var test = Optional.ofNullable(map.get(array[1]))
        .orElseGet(() -> Integer.parseInt(array[1]));

[Piet Souris]

I used what Tim suggested:

var test = map.contains(array[1]) ? map.get(array[1]) : Integer.parseInt(array[1]));

but Rob, that is indeed very nice! Deserves a cow!

I translated the input into an ArrayList<Runnable>, so in this case I had:

else if (arr[0].equals("jnz")) {
    var test = map.containsKey(arr[1]) ? map.get(arr[1]) : Integer.parseInt(arr[1]);
    instructions.add( () -> PC += test == 0 ? 1 : Integer.parseInt(arr[2]);
}
....

Edit:

@Rob,
any reason why you used orElseGet instead of orElse?

[Rob Spoor]

Piet Souris wrote:@Rob,
any reason why you used orElseGet instead of orElse?

The argument to orElse isn't lazily evaluated, so you'd have the same problem you originally had - array[1] is parsed even if it isn't necessary, and it fails for any non-number.