Allow to have different implementations of extensions

[jonenst]

Signed-off-by: Jon Harper jon.harper87@gmail.com

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

• The commit message follows our guidelines
• Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest
#828

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
It's not possible to have multiple implementations of an extension

What is the new behavior (if this is a feature change)?
it is possible to have multiple implementations of an extension

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:
NO

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

[jonenst]

Coverage is done is #1153

[mathbagu]

I request at least the adding of the license header

[mathbagu]

2 questions: do we allow null values and is extendable final?

[jonenst]

There is no problem with forbidding null values (this is always called by doing extendable.newExtensionAdder(..)) so the extendable is never null.
We can also make the extendable final, even private if we want (and pass it to the createExtension method.

You think we should do these changes ?

[mathbagu]

I think it's cleaner if you declare it private, final and if you assert the extendable is not null

[jonenst]

Will add a few tests in this PR by extracting the static state of ExtendableProviders

[jonenst]

I added tests in the same module as the code instead of relying on cross module testing. Also this improves the test coverage.

One problem with the current design is that it allows a compilation (with a warning) of the following code:

public void bad(VoltageLevel vl) {
  vl.newExtension(ConnectablePositionBuilder.class).add();//bad: compiles but shouldn't
}

The previous API was better (although not perfect)

public void bad(VoltageLevel vl) {
  vl.addExtension(ConnectablePosition.class, new ConnectablePosition<>());//good: compile error
  vl.addExtension(ConnectablePosition.class, new ConnectablePosition());//bad, compiles but shouldn't. But you are using raw types 
}

[jonenst]

The previous API also allowed to compile the following:

        vl.getExtension(ConnectablePosition.class); // should not compile; there is a warning though

I think we have to use the same trick as Collections.emptyList() vs Collections.EMPTY_LIST to allow the type checker to check the generic types

[jonenst]

@mathbagu What do you think of my proposed "clazz" static method ? It fixes the holes in the previous and new APIS.

It is only needed for generic extensions and generic extension adders, but should we say that every extension and extension adder should implement it ?

[jonenst]

We can also discuss to find a better solution to the asymetry introduced by the new API:

// before it was symmetric:
   extendable.addExtension(ExtensionAdder.class, ...);
   extendable.getExtension(Extension.class);

//now it is asymetric
  extendable.newExtension(ExtensionAdder.class);
  extendable.getExtension(Extension.class);

[mathbagu]

It's dirty but I assume we can write dirty code in tests... Default should not be used to avoid duplications

[jonenst]

The previous API also allowed to compile the following:

        vl.getExtension(ConnectablePosition.class); // should not compile; there is a warning though

I think we have to use the same trick as Collections.emptyList() vs Collections.EMPTY_LIST to allow the type checker to check the generic types

This is only in eclipse. Using maven (ie javac) or intellij, we get the expected compile error.

[geofjamg]

Ok apart some cosmetic Java & doc issues

[mathbagu]

@jonenst Could you check my last comment please?

[mathbagu]

Maybe a copy/past error: are you sure about this message?

[jonenst]

Oops yes

[jonenst]

fixed

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
2 Code Smells

90.0% Coverage
0.0% Duplication