Inconsistent method naming in LicenseCompareHelper
Nessuno ha ancora preso questa issue.
- 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:
- Whether the method operates on licenses, or exceptions
- Whether the method performs exact matching of the text, or "finding" within the text
- 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)
- Whether the method returns the first match, or all matches
- 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
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- 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