Suggest improvement to Util classes (Ignite ML)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Suggest improvement to Util classes (Ignite ML)

Mark Andreev
Hi, @Alexey Zinoviev <[hidden email]>.

I suggest to add a final class modifier and to add a private constructor
to Util classes in ignite ml. This is Sonar rule RSPEC-1118 (
https://rules.sonarsource.com/java/tag/design/RSPEC-1118).

Motivation:
Utility classes, which are collections of static members, are not meant to
be instantiated. Even abstract utility classes, which can be extended,
should not have public constructors. Java adds an implicit public
constructor to every class which does not define at least one explicitly.
Hence, at least one non-public constructor should be defined.

We can add this to:
- DistributedMetaStorageUtil.java
- ComputeUtils.java
- IgniteModelStorageUtil.java
- MapUtil.java
- MatrixUtil.java
- Utils.java
Class JdbcThinSSLUtil.java already has a private constructor.

--
Best regards,
Mark Andreev
Reply | Threaded
Open this post in threaded view
|

Re: Suggest improvement to Util classes (Ignite ML)

Alexey Zinoviev
Hi, Mark, it's great to hear from you a few suggestions related to code
culture and so on.
I hope during PR preparation it will be more of them and I suggest to
collect them in the shared doc (i've added first two of them)
https://docs.google.com/document/d/1_oBgmNfu6YnuSxEg9e1ImyGSV-fgmHq4Ut-hPq2bakQ/edit?usp=sharing

When we collect a few suggestions there I'll promise to spend time to solve
each incident to answer you or create a ticket or quick fix.
Also we could arrange a call in the next few weeks to answer your questions
and discuss your suggestions.

Sincerely
Alex

вт, 25 авг. 2020 г. в 22:17, Mark Andreev <[hidden email]>:

> Hi, @Alexey Zinoviev <[hidden email]>.
>
> I suggest to add a final class modifier and to add a private constructor
> to Util classes in ignite ml. This is Sonar rule RSPEC-1118 (
> https://rules.sonarsource.com/java/tag/design/RSPEC-1118).
>
> Motivation:
> Utility classes, which are collections of static members, are not meant to
> be instantiated. Even abstract utility classes, which can be extended,
> should not have public constructors. Java adds an implicit public
> constructor to every class which does not define at least one explicitly.
> Hence, at least one non-public constructor should be defined.
>
> We can add this to:
> - DistributedMetaStorageUtil.java
> - ComputeUtils.java
> - IgniteModelStorageUtil.java
> - MapUtil.java
> - MatrixUtil.java
> - Utils.java
> Class JdbcThinSSLUtil.java already has a private constructor.
>
> --
> Best regards,
> Mark Andreev
>