[jira] [Created] (IGNITE-13531) [ML] Code cleanup in Util classes

["Alexey Zinoviev (Jira)" <ji...@apache.org>]

Alexey Zinoviev created IGNITE-13531:
----------------------------------------

             Summary: [ML] Code cleanup in Util classes
                 Key: IGNITE-13531
                 URL: https://issues.apache.org/jira/browse/IGNITE-13531
             Project: Ignite
          Issue Type: Improvement
            Reporter: Alexey Zinoviev
             Fix For: 2.10


*Suggest improvement to Util classes*

 

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.

 

*Suggest add Serializable to Blas class*

I found that class Blas (org.apache.ignite.ml.math) is not Serializable but

fields f2jBlas and nativeBlas are transient. So I suggest adding

a Serializable to Blas class.

 

*Add final modifier to static inner fields in utils class*

Motivation:
This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

 

For example replace:

public static IgniteDifferentiableDoubleToDoubleFunction SIGMOID = new IgniteDifferentiableDoubleToDoubleFunction() {

}

With:
public static final IgniteDifferentiableDoubleToDoubleFunction SIGMOID = new IgniteDifferentiableDoubleToDoubleFunction() {

}

 

*Inefficient use of keySet iterator instead of entrySet*

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.

 

Possible problem is expected order for set.

 

For example:
for (Integer bucket : hist.keySet()) {

            accum += hist.get(bucket);

            res.put(bucket, accum);

}

 

*Can be replaced with single Arrays.fill method call*


For example:
for (int i = 0; i < mins.length; i++)

                mins[i] = Double.POSITIVE_INFINITY;

Can be replaced with:
Arrays.fill(mins, Double.POSITIVE_INFINITY);

Founded in:
 * ImputerTrainer
 * MaxAbsScalerTrainer
 * MinMaxScalerTrainer



--
This message was sent by Atlassian Jira
(v8.3.4#803005)