reduce public footprint of ev contrib

[kainagel]

bringing this closer to matsim standards (e.g. all static methods combined into one *Utils class; replace constructors by static factory methods in that *Utils class; then make implementing classes package-protected; make injected constructors non-public so that classes can only be instantiated through injection; ...).

can we still do this or is it too late (i.e. too many users)?

(If you prefer having the factory in the implementing class rather than in the *Utils class, that would be fine with me, and maybe even a bit more logical in the long run. Everywhere where it was like that I have left it like that, and we could also do it for the other implementing classes. It is just a little harder to audit, since one has to look into the class to make sure that it does not have a public constructor.)

[michalmac]

Thanks! It looks good to me.

Some thoughts:

1. I think making things package-protected and final is good in general. From time to time though, we are asked to remove some of these restrictions from some classes, because there is no better alternative. So I hope/assume we will get a feedback if we break someone else's code without leaving a reasonable workaround.

2. I often use factory methods inside a specific class/interface or interface-specific util (e.g. ElectricFleets), so things are more local and we avoid having big util classes (but maybe sometimes it leads to too many small utils here and there).

   I also think there is this naming policy for util classes to suffix them with s (e.g. Collectors, Arrays, Collections, Executors, Files or Objects).

   However, I do not have strong opinions on these (Utils as a suffix is fine, having factory methods in utils is fine).

[michalmac]

Regarding .asEagerSingleton() vs .in(Scopes.SINGLETON):

I mostly use the former, and I do not remember I've ever run into any issues with that. This is maybe related to the fact how I write tests:

• simple unit tests, where there is no DI (guice), because I try to keep such tests as simple as possible
• more complex tests (often suffixed with IT), where a proper matsim scenario is set up and then DI (guice) is used

I guess eager singletons may cause problems in "half-setup" scenarios ("medium complexity unit tests"). Or maybe there are some other cases where it also causes problems? Out of curiosity, do we have such issues mentioned/reported somewhere?

Anyway, I do not see any reason against switching to latter type. I can do it also in the dvrp-related contribs.

[kainagel]

Discussion re guice singletons moved to here: https://docs.google.com/document/d/1CSvPcMKrPy-0j_RBvMEyCyqqDq_JH1snFkGUlTrLIKM/edit#bookmark=id.b0vhu3gcrbgc .

[kainagel]

Discussion re constructors/factories moved to here: https://docs.google.com/document/d/1CSvPcMKrPy-0j_RBvMEyCyqqDq_JH1snFkGUlTrLIKM/edit#bookmark=id.nzodxwmhc9gb

[nkuehnel]

@kainagel @michalmac
We do have some troubles now with some classes, especially the default implementation of ChargingInfrastructureSpecification which has no public access anymore. This makes using the ChargerReader and manipulating chargers by code a bit cumbersome. At the moment we actually just duplicate the default implementation to our own repos which is not the desired outcome of this change, I guess. Couldn't we offer a public factory method in the Utils class as well? This should probably go to the existing ChargingInfrastructureUtils class

I can prepare a PR for that

[michalmac]

Couldn't we offer a public factory method in the Utils class as well? This should probably go to the existing ChargingInfrastructureUtils class

Sounds good.