0

I recently made it through to the final round of interview.

At one point in the interview, they asked me to demonstrate my Java 8 knowledge on the following piece of code. They asked me to reduce the following code using either Optional.of() or Stream.of(). And I was completely frozen, I only ever used streams on lists and didn't know how to use the optional approach. I didn't get the job specifically for this reason, as they said my understanding of java8 wasn't good enough. Can someone please tell me what they were looking for?

Summary

I've been specifically asked to reduce these 2 lines with Optional.of() or Stream.of():

gameDto = gameplay.playRandomGame(gameDto);
repo.updateTotals(gameDto.getResult());

Overall snippet for a bit of context:

@Service("gameService")
public class GameServiceImpl implements GameService{
    
    @Autowired
    private SessionInMemoryRegistry sessionRegistry;
    
    @Autowired
    private GameInMemoryRepo repo;
    
    @Autowired
    private GamePlay gameplay;

    @Override
    public ResponseDto addGameToSession(GameDto gameDto) {
        gameDto = gameplay.playRandomGame(gameDto);
        repo.updateTotals(gameDto.getResult());
        return sessionRegistry.addGameSession(gameDto.getSessionId(), gameDto.getPlayer1Choice(), gameDto.getPlayer2Choice(), gameDto.getResult());
    }
}
Makyen
  • 31,849
  • 12
  • 86
  • 121
r.lally91
  • 23
  • 2

1 Answers1

7

First of all, this code is fishy: @Autowired on fields, reassigned method argument.

And if you ask my opinion, this code doesn't need neither Streams, no Optional (since there's no method involved which return one). It seems like you were asked to rewrite the code in an obscuring way so that it would abuse either an Optional or Stream.

Number of lines is not a metric for code quality.

Here's an example of a Stream-abuse:

@Override
public ResponseDto addGameToSession(GameDto gameDto) {
    
    Stream.of(gameDto)
        .map(gameplay::playRandomGame)
        .forEach(gDto -> repo.updateTotals(gDto.getResult()));
    
    return sessionRegistry.addGameSession(gameDto.getSessionId(), gameDto.getPlayer1Choice(), gameDto.getPlayer2Choice(), gameDto.getResult());
}

The code above unnecessarily operates via side-effects, which is disparaged be the Stream API documentation.

And here's an example of an Optional-abuse (it can be more frequently observed with Optional.ofNullable()):

@Override
public ResponseDto addGameToSession(GameDto gameDto) {
    
    Optional.of(gameDto)
        .map(gameplay::playRandomGame)
        .ifPresent(gDto -> repo.updateTotals(gDto.getResult()));
    
    return sessionRegistry.addGameSession(gameDto.getSessionId(), gameDto.getPlayer1Choice(), gameDto.getPlayer2Choice(), gameDto.getResult());
}

In a nut-shell, Optional is a container of data, which is intended to represent a return type. You can wrap it around something nullable and that would allow to interact safely with a result of a method call regardless whether it contains the actual value or not.

Creation an Optional only in order chain methods on it, goes against its design goal. It's a smell and should be avoided.

Regarding the usage of Optional, see this and this answers by @Stuart Marks, Java and OpenJDK developer.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
  • 1
    Second example should be `Optional.of`, I agree this was just a "do you know how to `.map`" question – Caleth Oct 25 '22 at 09:38
  • You are right, they weren't implying this code was a good candidate for streams/optional, they were just seeing if I knew how to do it (I didn't!). So thanks a million for showing me what they wanted! – r.lally91 Oct 25 '22 at 09:44
  • 2
    @r.lally91 I assume that maybe it was kind of "double dip" question. The interviewer possibly wanted not only to find out are you familiar with functional programming features of Java, but also to test your ability to evaluate the code you've just written. – Alexander Ivanchenko Oct 25 '22 at 09:52
  • 1
    Letting aside that this is using the APIs in an unintended way, the interviewer’s task was “*to reduce these 2 lines*” and this result will only be one line if we make the line very long. It’s not as if we couldn’t write the original `gameDto = gameplay.playRandomGame(gameDto); repo.updateTotals(gameDto .getResult());` in a single line too; it’s even shorter. We can even write it as a single *statement* if that syntactic peculiarity was the goal: `repo.updateTotals((gameDto = gameplay.playRandomGame( gameDto)).getResult());` (it even works if getting questionable code was the actual goal). – Holger Oct 26 '22 at 07:49