Remove dependency to commons-collections4 (fixes #868) by reschke · Pull Request #870 · ESAPI/esapi-java-legacy

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can agree with testing; however, I'd like to throw out that if we're not tied to a Vector collection type, then the java 8 streams API can both consolidate the code and possibly clarify the desired behavior in edge-case scenarios:

    /**
     * Convert an array of fully qualified class names into an array of Class objects
     * @param parameterClassNames
     * @return The Class objects found that match the specified class names provided.
     */
    protected final Class[] getParameters(String[] parameterClassNames) {
        if (parameterClassNames == null) {
            return new Class[0];
        }

        List<Class> classes = Arrays.stream(parameterClassNames)
        .filter(Objects::nonNull)
        .filter(el -> !el.toString().trim().isEmpty()).map(name -> getClass(name, "parameter"))
        .collect(Collectors.toList());
        
      
        return classes.toArray(new Class[classes.size()]);
    }
  • testNullParameter //Empty Array
  • testArrayWithNullMember //Empty Array
  • testArrayWithBlankMember //Empty Array
  • testArrayWithInvalidClassName // ClassNotFoundException?
  • testArrayWithMixedMembers //contains (Null, empty, valid) --> Returns array of 1 with valid class

I don't know what tests currently exist, but those are the ones that I might consider in this update. I'm not sure of the complexity of stubbing the return from the getClass call either.