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)