Cannot Get Optimal Solution with 16 nodes of VRP with Time Windows. "Out of the box," the NetBeans Inspect mechanism finds a bunch of my code missing Javadoc statements, but does not flag the constructors and methods with too many parameters. How are we doing? These situations usually denote the need for new objects to wrap the numerous parameters.". The first seven posts of my series of dealing with too many parameters expected in Java methods focused on alternative approaches to reduce the number of parameters a method or constructor expects. Correctness - The type of a supplied argument doesn't match format specifier, Correctness - Uncallable method defined in anonymous class, Correctness - Uninitialized read of field in constructor, Correctness - Uninitialized read of field method called from constructor of superclass, Correctness - Unnecessary type check done using instanceof operator, Correctness - Unneeded use of currentThread() call, to call interrupted(), Correctness - Use of class without a hashCode() method in a hashed data structure, Correctness - Useless assignment in return statement, Correctness - Useless control flow to next line, Correctness - Using pointer equality to compare different types, Correctness - Vacuous call to collections, Correctness - Value annotated as carrying a type qualifier used where a value that must not carry that qualifier is required, Correctness - Value annotated as never carrying a type qualifier used where value carrying that qualifier is required, Correctness - Value is null and guaranteed to be dereferenced on exception path, Correctness - Value required to have type qualifier, but marked as unknown, Correctness - Value that might carry a type qualifier is always used in a way prohibits it from having that type qualifier, Correctness - Value that might not carry a type qualifier is always used in a way requires that type qualifier, Correctness - Very confusing method names, Correctness - close() invoked on a value that is always null, Correctness - equals method always returns false, Correctness - equals method always returns true, Correctness - equals method compares class names rather than class objects, Correctness - equals method overrides equals in superclass and may not be symmetric, Correctness - equals() method defined that doesn't override Object.equals(Object), Correctness - equals() used to compare array and nonarray, Correctness - hasNext method invokes next, Correctness - instanceof will always return false, Correctness - int value cast to double and then passed to Math.ceil, Correctness - int value cast to float and then passed to Math.round, Dead store due to switch statement fall through, Dead store due to switch statement fall through to throw, Dead store to local variable that shadows field, Dodgy - Ambiguous invocation of either an inherited or outer method, Dodgy - Check for oddness that won't work for negative numbers, Dodgy - Class doesn't override equals in superclass, Dodgy - Class exposes synchronization and semaphores in its public interface, Dodgy - Class extends Servlet class and uses instance variables, Dodgy - Class extends Struts Action class and uses instance variables, Dodgy - Class implements same interface as superclass, Dodgy - Class is final but declares protected field, Dodgy - Code contains a hard coded reference to an absolute pathname, Dodgy - Complicated, subtle or wrong increment in for-loop, Dodgy - Computation of average could overflow, Dodgy - Dead store of null to local variable, Dodgy - Dereference of the result of readLine() without nullcheck, Dodgy - Double assignment of local variable, Dodgy - Exception is caught when Exception is not thrown, Dodgy - Immediate dereference of the result of readLine(), Dodgy - Invocation of substring(0), which returns the original value, Dodgy - Method checks to see if result of String.indexOf is positive, Dodgy - Method directly allocates a specific implementation of xml interfaces, Dodgy - Method discards result of readLine after checking if it is nonnull, Dodgy - Method uses the same code for two branches, Dodgy - Method uses the same code for two switch clauses, Dodgy - Non serializable object written to ObjectOutput, Dodgy - Non-Boolean argument formatted using %b format specifier, Dodgy - Parameter must be nonnull but is marked as nullable, Dodgy - Possible null pointer dereference due to return value of called method, Dodgy - Possible null pointer dereference on path that might be infeasible, Dodgy - Potentially dangerous use of non-short-circuit logic, Dodgy - Questionable cast to abstract collection, Dodgy - Questionable cast to concrete collection, Dodgy - Questionable use of non-short-circuit logic, Dodgy - Redundant comparison of non-null value to null, Dodgy - Redundant comparison of two null values, Dodgy - Redundant nullcheck of value known to be non-null, Dodgy - Redundant nullcheck of value known to be null, Dodgy - Remainder of 32-bit signed random integer, Dodgy - Remainder of hashCode could be negative, Dodgy - Result of integer multiplication cast to long, Dodgy - Self assignment of local variable, Dodgy - Thread passed where Runnable expected. Identifying a novel about floating islands, dragons, airships and a mysterious machine. I don't think there's any set maximum number of parameters, but seven does seem like a "rule of thumb" to rarely exceed and I do generally prefer a smaller number such as Martin's recommendation of fewer than three arguments. US to Canada by car with an enhanced driver's license, no passport? Why had climate change not been proven beyond doubt for so long? Dodgy - Transient field of class that isn't Serializable. |. Is a neuron's information processing more complex than a perceptron? Correctness - The readResolve method must not be declared as a static method. What would the ancient Romans have called Hercules' Club? As I have been writing this portion of this post, I'm reminded that NetBeans provides significant static code analysis support. NetBeans PMD support is available via the PMD Plugin. I can name the new configuration "Java Code Metrics". in case of a method: with a request parameter and a builder if has too many parameters. Any tool must have a specified number of parameters that is considered "too many." in case of a constructor: a builder pattern. Please help us improve Stack Overflow. The next screen snapshot demonstrates use of Java Code Metrics in NetBeans 7.4. Bad practice - Unchecked type in generic call, Bad practice - Usage of GetResource may be unsafe if class is extended, Bad practice - Use of identifier that is a keyword in later versions of Java, Bad practice - Very confusing method names (but perhaps intentional), Bad practice - equals method fails for subtypes, Bad practice - equals() method does not check for null argument, Bad practice - serialVersionUID isn't final, Bad practice - serialVersionUID isn't long, Bad practice - serialVersionUID isn't static, Bad practice - toString method may return null, BigDecimal constructed from double that isn't represented precisely, Boxed value is unboxed and then immediately reboxed. Clicking on the "Default" label leads to a drop-down from which "New" can be selected. It's likely that yet more parameters will be added in the future, so the parameters should be grouped into a Command Object, improving maintenance costs. Static analysis tools can be used to automatically identify methods or constructors which might expect too many parameters. Is the fact that ZFC implies that 1+1=2 an absolute truth? To learn more, see our tips on writing great answers. *" packages should not be used, Classes named like "Exception" should extend "Exception" or a subclass, Classes that override "clone" should be "Cloneable" and call "super.clone()", Classes with only "static" methods should not be instantiated, Collapsible "if" statements should be merged, Collection.isEmpty() should be used to test for emptiness, Collections should not be passed as arguments to their own methods, Collections.emptyList(), emptyMap() and emptySet() should be used instead of Collections.EMPTY_LIST, EMPTY_MAP and EMPTY_SET, Conditions in related "if/else if" statements should not have the same condition, Constant names should comply with a naming convention, Constants should not be defined in interfaces, Control flow statements "if", "for", "while", "switch" and "try" should not be nested too deeply, Cryptographic RSA algorithms should always incorporate OAEP (Optimal Asymmetric Encryption Padding), Custom serialization method signatures should meet requirements, Declarations should use Java collection interfaces such as "List" rather than specific implementation classes such as "LinkedList", Deprecated code should be removed eventually, Deprecated elements should have both the annotation and the Javadoc tag, Dissimilar primitive wrappers should not be used with the ternary operator without explicit casting, Empty arrays and collections should be returned instead of null, Exception handlers should preserve the original exception, Exception types should not be tested using "instanceof" in catch blocks, Exceptions should not be thrown in finally blocks, Execution of the Garbage Collector should be triggered only by the JVM, Field names should comply with a naming convention, Fields in a "Serializable" class should either be transient or serializable, Fields in non-serializable classes should not be "transient", Floating point numbers should not be tested for equality, Future keywords should not be used as names, Generic exceptions should never be thrown, Generic wildcard types should not be used in return parameters, Identical expressions should not be used on both sides of a binary operator, IllegalMonitorStateException should not be caught, Inappropriate "Collection" calls should not be made, Inner class calls to super class methods should be unambiguous, Instance methods should not write to "static" fields, Insufficient branch coverage by unit tests, Interface names should comply with a naming convention, Interfaces should not have "public static" mutable fields, Ints and longs should not be shifted by more than their number of bits-1, Lambdas and anonymous classes should not have too many lines, Lamdbas containing only one statement should not nest this statement in a block, Literal boolean values should not be used in condition expressions, Local Variables should not be declared and then immediately returned or thrown, Local variable and method parameter names should comply with a naming convention, Local variables should not shadow class fields, Loggers should be "private static final" and should share a naming convention, Loop conditions should be true at least once, Loop invariants should not be calculated inside the loop, Loops should not contain more than a single "break" or "continue" statement, Math operands should be cast before assignment, Method names should comply with a naming convention, Method parameters, caught exceptions and foreach variables should not be reassigned, Methods "wait()", "notify()" and "notifyAll()" should never be called on Thread instances, Methods named "equals" should override Object.equals(Object), Methods should not be named "hashcode" or "equal", Methods should not have too many parameters, Modifiers should be declared in the correct order, Nested blocks of code should not be left empty, Non-constructor methods should not have the same name as the enclosing class, Non-public methods should not be "@Transactional", Non-serializable classes should not be written, Non-serializable objects should not be stored in "HttpSessions", Null should not be returned from a "Boolean" method, Object.finalize() should not be overloaded (by adding method parameters), Objects should not be created only to "getClass", Objects should not be created to be dropped immediately without being used, Only static class initializers should be used, Overriding methods should do more than simply call the same method in the super class, Package declaration should match source file directory, Package names should comply with a naming convention, Parentheses should be removed from a single lambda input parameter when its type is inferred, Parsing should be used to convert "Strings" to primitives, Primitive wrappers should not be instantiated only to perform a "toString" conversion, Primitives should not be boxed just for "String" conversion, Printf-style format strings should not lead to unexpected behavior at runtime, Public constants should be declared "static final" rather than merely "final", Public methods should throw at most one checked exception, Reflection should not be used to check non-runtime annotations, Relational operators should be used in "for" loop termination conditions, Return of boolean expressions should not be wrapped into an "if-then-else" statement, Sections of code should not be "commented out", Servlets should never have mutable instance fields, Short-circuit logic should be used in boolean contexts, Silly bit operations should not be performed, Standard ouputs should not be used directly to log anything, String.valueOf() should not be appended to a String, Strings literals should be placed on the left side when checking for equality, Subclasses that add fields should override "equals", Switch cases should end with an unconditional "break" statement, Synchronisation should not be based on Strings or boxed primitives, Synchronized classes Vector, Hashtable, Stack and StringBuffer should not be used, The Array.equals(Object obj) method should not be used, The Object.finalize() method should not be called, The Object.finalize() method should not be overriden, The default unnamed package should not be used, The members of an interface declaration or class should appear in a pre-defined order, The non-serializable super class of a "Serializable" class must have a no-argument constructor, The value returned from a stream read should be checked, Thread.run() and Runnable.run() should not be called directly, Throwable.printStackTrace() should not be called, Throws declarations should not be redundant, Type parameter names should comply with a naming convention, Unused method parameters should be removed, Useless parentheses around expressions should be removed to prevent any misunderstanding, Utility classes should not have public constructors, Values passed to SQL commands should be sanitized, Values should not be uselessly incremented, super.finalize() should be called at the end of Object.finalize() implementations, Adding elements of an entry set may fail due to reuse of Entry objects, An increment to a volatile field isn't atomic, Bad comparison of int value with long constant, Bad practice - Abstract class defines covariant compareTo() method, Bad practice - Certain swing methods needs to be invoked in Swing thread, Bad practice - Check for sign of bitwise operation, Bad practice - Class defines clone() but doesn't implement Cloneable, Bad practice - Class implements Cloneable but does not define or use clone method, Bad practice - Class inherits equals() and uses Object.hashCode(), Bad practice - Class is Externalizable but doesn't define a void constructor, Bad practice - Class is Serializable but its superclass doesn't define a void constructor, Bad practice - Class is Serializable, but doesn't define serialVersionUID, Bad practice - Class is not derived from an Exception, even though it is named as such, Bad practice - Class names shouldn't shadow simple name of implemented interface, Bad practice - Class names shouldn't shadow simple name of superclass, Bad practice - Classloaders should only be created inside doPrivileged block, Bad practice - Clone method may return null, Bad practice - Comparator doesn't implement Serializable, Bad practice - Comparison of String objects using == or !=, Bad practice - Comparison of String parameter using == or !=, Bad practice - Covariant compareTo() method defined, Bad practice - Creates an empty jar file entry, Bad practice - Creates an empty zip file entry, Bad practice - Dubious catching of IllegalMonitorStateException, Bad practice - Empty finalizer should be deleted, Bad practice - Equals checks for noncompatible operand, Bad practice - Equals method should not assume anything about the type of its argument, Bad practice - Fields of immutable classes should be final, Bad practice - Finalizer does not call superclass finalizer, Bad practice - Finalizer does nothing but call superclass finalizer, Bad practice - Finalizer nullifies superclass finalizer, Bad practice - Finalizer only nulls fields, Bad practice - Iterator next() method can't throw NoSuchElementException, Bad practice - Method doesn't override method in superclass due to wrong package for parameter, Bad practice - Method ignores exceptional return value, Bad practice - Method ignores results of InputStream.read(), Bad practice - Method ignores results of InputStream.skip(), Bad practice - Method invoked that should be only be invoked inside a doPrivileged block, Bad practice - Method invokes dangerous method runFinalizersOnExit, Bad practice - Method may fail to close database resource, Bad practice - Method may fail to close database resource on exception, Bad practice - Method may fail to close stream, Bad practice - Method may fail to close stream on exception, Bad practice - Method might drop exception, Bad practice - Method might ignore exception, Bad practice - Method with Boolean return type returns explicit null, Bad practice - Needless instantiation of class that only supplies static methods, Bad practice - Non-serializable class has a serializable inner class, Bad practice - Non-serializable value stored into instance field of a serializable class, Bad practice - Random object created and used only once, Bad practice - Static initializer creates instance before all static final fields assigned, Bad practice - Store of non serializable object into HttpSession, Bad practice - Superclass uses subclass during initialization, Bad practice - Suspicious reference comparison. To do this, I can click on the Source | Inspect and select "Default" for "Configuration.". The next two screen snapshots demonstrate that these values are set for constructors and methods respectively in the same window where we checked the options we wanted inspected. Multithreaded correctness - Synchronize and null check on the same field. In his blog post "Just How Messed Up Is My Code?," the "occasional" NetBeans blogger Geertjan Wielenga demonstrates how to configure the Java Code Metrics to be active. In many ways, software development has its own "eye tests" that tell us when certain things are better or worse than the "rules" imply. Modern Java IDEs are particularly helpful in the refactoring and code generation efforts. Dependency injection through constructors or property setters? The first image shows the high-level results and following images show more specific details made available by clicking on the high-level results. Once identified, these constructors and methods can be manually changed/refactored to reduce the number of parameters with approaches such as the ones I've outlined in earlier posts in this series of too many parameters. In this eighth post in the series, I look at tools that help identify cases where too many parameters may exist and tools that help deal with that when it occurs. This configuration is really easy with NetBeans's Java Code Metrics support. The Builder Pattern Eclipse Plugin is available for Eclipse. It includes Code Audit capabilities with one category of rules being "Program Complexity." The "Method with too many parameters" inspection is described as: "This inspection reports any instances of methods with too many parameters. What's the difference between the Dependency Injection and Service Locator patterns? In my case a Spring service needed a lot of other services in order to do its work. Constructors typically take more parameters than a regular method, especially when initializing a large object. Specifically, Checkstyle provides the ParameterNumber "check" with the description, "Checks the number of parameters of a method or constructor." ".equals()" should not be used to test the values of "Atomic" classes, "@Override" annotation should be used on any method overriding (since Java 5) or implementing (since Java 6) another one, "ConcurrentLinkedQueue.size()" should not be used, "Double.longBitsToDouble" should not be used for "int", "HttpServletRequest.getRequestedSessionId()" should not be used, "Iterator.hasNext()" should not call "Iterator.next()", "Iterator.next()" methods should throw "NoSuchElementException", "Lock" objects should not be "synchronized", "Object.finalize()" should remain protected (versus public) when overriding, "Object.wait()" and "Condition.await()" should always be called inside a "while" loop, "Object.wait()" should never be called on objects that implement "java.util.concurrent.locks.Condition", "PreparedStatement" and "ResultSet" methods should be called with valid indices, "ScheduledThreadPoolExecutor" should not have 0 core threads, "Serializable" inner classes of non-serializable classes should be "static", "StringBuilder" and "StringBuffer" should not be instantiated with a character, "Threads" should not be used where "Runnables" are expected, "URL.hashCode" and "URL.equals" should be avoided, "compareTo" results should not be checked for specific values, "compareTo" should not return "Integer.MIN_VALUE", "equals(Object obj)" and "hashCode()" should be overridden in pairs, "equals(Object obj)" should be overridden along with the "compareTo(T obj)" method, "finalize" should not set fields to "null", "for" loop incrementers should modify the variable being tested in the loop's stop condition, "hashCode" and "toString" should not be called on array instances, "if" statement conditions should not unconditionally evaluate to "true" or to "false", "instanceof" operators that always return "true" or "false" should be removed, "object == null" should be used instead of "object.equals(null)", "public static" fields should always be constant, "return" statements should not occur in "finally" blocks, "runFinalizersOnExit" should not be called, "static final" arrays should be "private", "switch case" clauses should not have too many lines, "switch" statements should end with a "default" clause, "switch" statements should have at least 3 "case" clauses, "switch" statements should not contain non-case labels, "switch" statements should not have too many "case" clauses, "toString()" and "clone()" methods should not return null, "wait()" should be used instead of "Thread.sleep()" when a lock is held, "wait()", "notify()" and "notifyAll()" methods should only be called when a lock is obviously held on an object, A "for" loop update clause should move the counter in the right direction, A close curly brace should be located at the beginning of a line, A field should not duplicate the name of its containing class, Array designators "[]" should be located after the type in method signatures, Array designators "[]" should be on the type, not the variable, Assignments should not be made from within sub-expressions, Boxing and unboxing should not be immediately reversed, Case insensitive string comparisons should be made without intermediate upper or lower casing, Class names should comply with a naming convention, Class names should not shadow interfaces or superclasses, Class variable fields should not have public accessibility, Classes extending java.lang.Thread should override the "run" method, Classes from "sun. Checkstyle is defined on its main web page as "a development tool to help programmers write Java code that adheres to a coding standard." rev2022.7.21.42639. Making statements based on opinion; back them up with references or personal experience. Class defines tostring(); should it be toString()? In the twin paradox or twins paradox what do the clocks of the twin and the distant star he visits show when he's at the star? One of PMD's rules is "ExcessiveParameterList" (LongParameterListRule in PMD 4.3 instead of ExcessiveParameterList). Class defines equal(Object); should it be equals(Object)? The tools discussed so far in this post have been useful in analyzing code to find existing methods and constructors expecting too many parameters. Selecting "Default" allows me to now click on the "Manage" button and clicking on that button presents the "Configuration" window. In Checkstyle's case, the default "maximum allowable number of parameters" for a constructor or method is 7 (same number as Steve McConnell's recommendation). This inspection allows the number of method parameters that is too many to be configured. Dodgy - Unsigned right shift cast to short/byte, Dodgy - Vacuous bit mask operation on integer value, Dodgy - Vacuous comparison of integer value, Dodgy - Write to static field from instance method, Dodgy - instanceof will always return true, Dodgy - int division result cast to double or float, Dodgy - private readResolve method not inherited by subclasses, Experimental - Potential lost logger changes due to weak reference in OpenJDK, Field isn't final but should be refactored to be so, Field names should start with a lower case letter, Format string should use %n rather than \n, Internationalization - Consider using Locale parameterized version of invoked method, Malicious code vulnerability - Field is a mutable Hashtable, Malicious code vulnerability - Field is a mutable array, Malicious code vulnerability - Field isn't final and can't be protected from malicious code, Malicious code vulnerability - Field should be both final and package protected, Malicious code vulnerability - Field should be moved out of an interface and made package protected, Malicious code vulnerability - Field should be package protected, Malicious code vulnerability - Finalizer should be protected, not public, Malicious code vulnerability - May expose internal representation by incorporating reference to mutable object, Malicious code vulnerability - May expose internal representation by returning reference to mutable object, Malicious code vulnerability - May expose internal static state by storing a mutable object into a static field, Malicious code vulnerability - Public static method may expose internal representation by returning array, Method may fail to clean up stream or resource on checked exception, Multithreaded correctness - A thread was created using the default empty run method, Multithreaded correctness - A volatile reference to an array doesn't treat the array elements as volatile, Multithreaded correctness - Call to static Calendar, Multithreaded correctness - Call to static DateFormat, Multithreaded correctness - Class's readObject() method is synchronized, Multithreaded correctness - Class's writeObject() method is synchronized but nothing else is, Multithreaded correctness - Condition.await() not in loop, Multithreaded correctness - Constructor invokes Thread.start(), Multithreaded correctness - Empty synchronized block, Multithreaded correctness - Field not guarded against concurrent access, Multithreaded correctness - Inconsistent synchronization, Multithreaded correctness - Incorrect lazy initialization and update of static field, Multithreaded correctness - Incorrect lazy initialization of static field, Multithreaded correctness - Method calls Thread.sleep() with a lock held, Multithreaded correctness - Method does not release lock on all exception paths, Multithreaded correctness - Method does not release lock on all paths, Multithreaded correctness - Method spins on field, Multithreaded correctness - Method synchronizes on an updated field, Multithreaded correctness - Mismatched notify(), Multithreaded correctness - Mismatched wait(), Multithreaded correctness - Monitor wait() called on Condition, Multithreaded correctness - Mutable servlet field, Multithreaded correctness - Possible double check of field, Multithreaded correctness - Static Calendar, Multithreaded correctness - Static DateFormat, Multithreaded correctness - Sychronization on getClass rather than class literal, Multithreaded correctness - Synchronization on Boolean could lead to deadlock, Multithreaded correctness - Synchronization on boxed primitive could lead to deadlock, Multithreaded correctness - Synchronization on boxed primitive values, Multithreaded correctness - Synchronization on field in futile attempt to guard that field, Multithreaded correctness - Synchronization on interned String could lead to deadlock, Multithreaded correctness - Synchronization performed on java.util.concurrent Lock. The Interleaving Effect: How widely is this used? Bad practice - The readResolve method must be declared with a return type of Object. QAPlug-PMD is a similar plug-in for IntelliJ IDEA and PMD Eclipse is available for Eclipse. Short story about the creation of a spell that creates a copy of a specific woman. As the last series of screen snapshots demonstrate, NetBeans 7.4 allows us to specifically inspect code for methods and constructors that have "too many parameters." So rule squid:S00107 fired and gave me an error in the SonarCube. Did Sauron suspect that the Ring would be destroyed? For example, in software, we might say that "fewer parameters is generally better than more parameters." Why is the US residential model untouchable and unquestionable? Pressing "Inspect" to apply the newly created "Java Code Metrics" inspection, leads to results shown in the following screen snapshots. For my demonstration purposes, I select "All Analyzes" and then click on the "Inspect" button. Is it possible that sonarlint doesn't show the "too many parameters" code smell warning for constructors that use injection? Newer PMD documentation puts it this way, "Methods with numerous parameters are a challenge to maintain, especially if most of them share the same datatype. Another newly added hint, "Method declares too many parameters," is described as, "Reports method that take too many parameters. In other words, in a way that is difficult to describe, the viewer feels the team or player is not as skilled as their statistics imply.