AbstractContextBuilder.set(Class<T>, T) implementation is weird

[marschall]

I had a look at the implementation of AbstractContextBuilder.set(Class, T) and I'm a bit confused:

    public <T> B set(Class<T> key, T value) {
        B old = set(key.getName(), Objects.requireNonNull(value));
        if (old != null && old.getClass().isAssignableFrom(value.getClass())) {
            return old;
        }
        return (B) this;
    }

It calls #set(String, Object), however this method does not return the old value but this so "old" is a confusing name. Then it makes tests whether "old" (which is this) is a super type of the value class. If it is it returns "old", which is this, otherwise it returns this.

I believe the method should just be:

    public <T> B set(Class<T> key, T value) {
        return set(key.getName(), Objects.requireNonNull(value));
    }

[keilw]

This does not seem to be a showstopper or have significant impact on the outside for users of the API, does it?

[marschall]

No, not at all. It’s just a clean up or code quality thing that affects only the implementation. The behavior users see is exactly the same.