spdx / spdx/Spdx-Java-Library

Inconsistent method naming in LicenseCompareHelper

Aperta
#392 4 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

matching
Lingua principale
Java
Stelle
71
Fork
44
Merge medio
12h 54m
PR unite (30g)
7

Descrizione

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 #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.

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia con la classe org.spdx.utility.compare.LicenseCompareHelper e il relativo Javadoc, quindi fai un inventario delle signature dei metodi esistenti di matching e diff rispetto alla matrice proposta. Esamina come vengono utilizzati i metodi attuali prima di definire nomi coerenti, deleghe di compatibilità e deprecazioni. Il lavoro è considerato completato quando lo schema di denominazione è implementato senza rimuovere i vecchi metodi prima di una major release.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
java
Ambito
backend
Tipo di issue
Refactoring
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Ferma
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.