Inconsistent method naming in LicenseCompareHelper

[pmonks]

Problem Statement

I've found that the names of the various different permutations of the matching methods in the org.spdx.utility.compare.LicenseCompareHelper class are pretty confusing, and wonder if a more consistent naming scheme would be valuable?

This is partly fueled by:

• There is no overall pattern to how the various methods are named, making "at a glance inference of method variations" difficult.
• There are some missing permutations (e.g. see Missing method in LicenseCompareHelper: matchingStandardLicenseExceptionIds #391).
• There are inconsistencies in how exceptions are referred to (some methods use "LicenseException", while others use "Exception").
• There's conflation of "pure" matching methods (i.e. methods that answer the question "did this text contains one or more licenses / exceptions, and (optionally) if so, which ones?") with the methods that calculate diffs (i.e. methods that answer the question "what, if any, differences exist between this text and a given license / exception?").

It seems to me that the methods in this class fall into a sparse matrix defined by these criteria:

1. Whether the method operates on licenses, or exceptions
2. Whether the method performs exact matching of the text, or "finding" within the text
3. Whether the method considers only one listed license / exception, or multiple listed licenses / exceptions (and in the latter case, whether that's all listed licenses / exceptions, or a subset provided by the caller)
4. Whether the method returns the first match, or all matches
5. Whether the method is concerned with "pure" matching, or calculating differences

Coming up with a systematic naming scheme informed by these criteria has substantial value in helping to communicate how the LicenseCompareHelper class can be used for different matching / diffing scenarios.

A Proposed Naming Scheme

Methods that perform exact matching of the entire text:

Method Signature	Notes
boolean isLicense(ListedLicense lic, String text)
boolean isException(ListedLicenseException exc, String text)
String isOneOfTheseLicenses(List<ListedLicense> lics, String text)	Returns the id of the first listed license in lics that matched, or null if no match was found
String isOneOfTheseExceptions(List<ListedLicenseException> excs, String text)	Returns the id of the first listed exception in excs that matched, or null if no match was found
String isAnyLicense(String text)	Returns the id of the first listed license that matched, or null if no match was found
String isAnyException(String text)	Returns the id of the first listed exception that matched, or null if no match was found
List<String> listAllLicensesMatched(List<ListedLicense> lics, String text)	Returns the ids of all of the listed licenses in lics that matched, or null if no match was found. Might not be necessary, as this is a corner case for overlapping matching templates (e.g. GPL).
List<String> listAllExceptionsMatched(List<ListedLicenseException> excs, String text)	Returns the ids of all of the listed exceptions in excs that matched, or null if no match was found. Might not be necessary, as this is a corner case for overlapping matching templates (e.g. GPL).
List<String> listAllLicensesMatched(String text)	Returns the ids of all of the listed licenses that matched, or null if no match was found. Might not be necessary, as this is a corner case for overlapping matching templates (e.g. GPL).
List<String> listAllExceptionsMatched(String text)	Returns the ids of all of the listed exceptions that matched, or null if no match was found. Might not be necessary, as this is a corner case for overlapping matching templates (e.g. GPL).

Methods that perform finds within the text, but only return the first match:

Method Signature	Notes
boolean containsLicense(ListedLicense lic, String text)
boolean containsException(ListedLicenseException exc, String text)
String containsOneOfTheseLicenses(List<ListedLicense> lics, String text)	Returns the id of the first listed license in lics that matched, or null if no match was found
String containsOneOfTheseExceptions(List<ListedLicenseException> excs, String text)	Returns the id of the first listed exception in excs that matched, or null if no match was found
String containsAnyLicense(String text)	Returns the id of the first listed license that matched, or null if no match was found
String containsAnyException(String text)	Returns the id of the first listed exception that matched, or null if no match was found

Methods that perform finds within the text, and return all matches:

Method Signature	Notes
List<String> listAllLicensesFound(List<ListedLicense> lics, String text)	Returns the ids of all of the listed licenses in lics that matched, or null if no match was found
List<String> listAllExceptionsFound(List<ListedLicenseException> excs, String text)	Returns the ids of all of the listed exceptions in excs that matched, or null if no match was found
List<String> listAllLicensesFound(String text)	Returns the ids of all of the listed licenses that matched, or null if no match was found
List<String> listAllExceptionsFound(String text)	Returns the ids of all of the listed exceptions that matched, or null if no match was found

Methods that calculate diffs:

Method Signature	Notes
CompareTemplateOutputHandler.DifferenceDescription diffLicense(ListedLicense lic, String text)
CompareTemplateOutputHandler.DifferenceDescription diffException(ListedLicenseException exc, String text)

Notes:

• Many of these methods will share a common implementation - in fact in many cases they can be implemented as simple delegation calls to other methods (sometimes in a loop).
• Obviously the old method names can't be removed until the next major release of the library, but they could be re-implemented to simply delegate to the new methods, and marked as deprecated before then.
• I don't believe any of the method signatures listed above conflict with existing method signatures, but haven't done an exhaustive check.

[goneall]

Couple thoughts:

• In general, I support renaming the methods. Some of the existing methods reference "standard" licenses when "listed" licenses should be used. I like the proposed consistency in naming. A long time ago, some of us referred to the license list as "standard" licenses which many folks objected to, so we now use the term "listed" licenses.
• I don't support separating out the diff methods from the matching methods. Since the matching code collects diffs without any additional compute cost (at least today), I would prefer to return the full diff information which is easily queried to find out if it matches. If they were separate, it would lead to people calling the match then immediately calling the diff if it didn't match which would cost 2X performance.
• We should qualify that the licenses being compared are listed licenses - there are other methods that support comparing any 2 license texts which may be confusing if we don't include the "listed" qualifier in the name.