Add additional methods to interact with an ArgumentValue

[JBodkin-Amphora]

No description provided.

[bclozel]

Thanks very much @JBodkin-Amphora for your first contribution to the project.
I took the liberty to add some tests, fix the isEmpty implementation, and remove the ifPresentOrEmpty as I don't think it's worth adding to our public API at this stage.

[JBodkin-Amphora]

Thank you for taking a look at this, the ifPresentOrEmpty method is the most important for us as it helps to reduce the verbosity of the code.

For example, using if else if is the most verbose:

if (message.isPresent()) {
	entity.setMessage(entityManager.find(...));
} else if (message.isEmpty()) {
	entity.setMessage(null);
}

The verbosity could be reduced by checking for not omitted, however we have an intermediate operation to look for an entity reference. In this instance, we're not using the data integration as the lookup for the entity has additional requirements that need to match but not demonstrated here. Also, using optionals potentially has overhead as they may not be optimised by the compiler.

For example:

if (!message.isOmitted()) {
	final var value = message.asOptional().map(entityManager.find()).orElse(null);
	entity.setMessage(value);
}

If we had the ifPresentOrEmpty method, it would look more readable:

message.ifPresentOrEmpty(
	value -> entity.setMessage(entityManager.find(...)),
	entity.setMessage(null)
);

What do you think?

[bclozel]

Thanks for the feedback. I initially thought that the main driver for this contribution was the testing bits of your other PR.
The two other variants don't look bad to me and I don't understand the point about Optional and compiler optimization as ArgumentValue is likely to be the same (or worse if Optional has intrisics).

Overall I'm not sure we should expand the ArgumentValue API, especially if we're getting closer to the Optional API with subtle semantic differences. We can reconsider this if we get more demand from the community.

[JBodkin-Amphora]

If we create a ArgumentValue and a Optional, then we're creating 2x the number of objects which could make it onto the heap and require garbage collection. We have some particularly large models which could end up with a few hundred fields in a mutation, so I'm trying to think ahead to keep the number of objects created to a minimum whilst ensuring the readability of the code. We're going through a transitional project to move to GraphQL (rather than transformational) which means we don't have the scope to make changes to the inputs, as we're keeping them as-is.

I understand that your not keen on supplementing the ArgumentValue API at the moment, so internally we can implement our own alternative and register our own ArgumentResolver.

[bclozel]

I'll discuss this with the rest of the team.