What we are planning is that all of it happens in a single function/class (or rather as a single responsibility).
Non posso dire che abbia mai pianificato una responsabilità. Trovo che pianifico il comportamento e le responsabilità emergano man mano che quel comportamento viene assegnato o implementato.
Nel tuo caso, il comportamento pianificato è che i due componenti riceveranno chiavi in cui i valori soddisfano determinate condizioni da un determinato set di oggetti JSON. Gli oggetti iniziali che abbiamo sono i componenti, i valori JSON e alcuni nuovi comportamenti che copriranno il resto. L'implementazione più semplice (assumendo alcuni dettagli su come funzionerebbe):
void extractKeysForComponents(List<JsonValue> jsonValues, Component component1, Component component2) {
jsonValues.stream()
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component1.add(key));
jsonValues.stream()
.filter((jsonValue) -> jsonValue.value() instanceof Number)
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component2.add(key));
}
La nostra implementazione più semplice implementa il comportamento che vogliamo. Ovviamente abbiamo anche scritto dei test per questo (che presumo siano lì). Questo metodo è però molto puzzolente. Il metodo accetta due parametri Component che non interagiscono. Questa è la prima indicazione che stiamo facendo più di una cosa. L'implementazione ha due parti chiaramente distinte. Questa è la seconda indicazione. Queste due parti sono anche quasi duplicati. Questa è la nostra terza indicazione. A questo punto, possiamo vedere che potrebbe esserci più di una responsabilità nascosta lì dentro, ma ciò che sono non è ancora emerso. Sappiamo che la duplicazione è il peggiore dei tre indicatori che abbiamo, ma abbiamo solo near -duplication. Facciamoli il più possibile simili:
void extractKeysForComponents(List<JsonValue> jsonValues, Component component1, Component component2) {
jsonValues.stream()
.filter((jsonValue) -> true)
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component1.add(key));
jsonValues.stream()
.filter((jsonValue) -> jsonValue.value() instanceof Number)
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component2.add(key));
}
Ora entrambe le parti sono simili come possiamo farle conservando il comportamento. Fissare la quasi duplicazione ci indica anche una potenziale responsabilità: il filtraggio. Prima avevamo un filtro predefinito nascosto senza filtro. Ora, questo è diventato esplicito. Esaminiamo ciò che varia in modo che possiamo iniziare a incapsularlo:
void extractKeysForComponents(List<JsonValue> jsonValues, Component component1, Component component2) {
Predicate<JsonValue> component1Filter = (jsonValue) -> true;
Predicate<JsonValue> component2Filter = (jsonValue) -> jsonValue.value() instanceof Number;
jsonValues.stream()
.filter(component1Filter)
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component1.add(key));
jsonValues.stream()
.filter(component2Filter)
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component2.add(key));
}
Ora vediamo che l'unica differenza tra le parti duplicate è quale componente e filtro associato sono usati. Forse possiamo usare una struttura dati per associare questi due componenti ai loro filtri in modo generico:
void extractKeysForComponents(List<JsonValue> jsonValues, Component component1, Component component2) {
Map<Component, Predicate<JsonValue> componentsAndPredicates = new LinkedHashMap<>();
componentsAndPredicates.put(component1, (jsonValue) -> true);
componentsAndPredicates.put(component2, (jsonValue) -> jsonValue.value() instanceof Number);
for (Map.Entry<Component, Predicate<JsonValue>> entry : componentsAndPredicates) {
jsonValues.stream()
.filter(entry.getValue())
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> entry.getKey().add(key));
}
}
Ora le responsabilità separate cominciano più chiaramente ad emergere. Component s sono associati a filtri specifici, quindi forse dovremmo spostare quei filtri nelle classi Component ? Ogni componente può quindi fornire il suo filtro in un modo generico. (Questo è un modo per affrontare questo problema e ce ne sono altri. Sto semplicemente scegliendo questo perché non ho abbastanza dettagli sul tuo codice specifico.) Il codice dovrebbe essere simile a:
void extractKeysForComponents(List<JsonValue> jsonValues, Component component1, Component component2) {
List<Component> components = Arrays.asList(component1, component2);
for (Component component : components) {
jsonValues.stream()
.filter(component.getJsonValueFilter())
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component.add(key));
}
}
Ciò che ora è più strano è che prendiamo solo due% di parametri di% di conversione per convertirli in un elenco e gestirli genericamente. Nella programmazione, ci sono solo tre numeri: zero, uno e molti . Possiamo prendere un numero arbitrario di Component s e gestirli tutti ugualmente bene.
void extractKeysForComponents(List<JsonValue> jsonValues, Component... components) {
for (Component component : components) {
jsonValues.stream()
.filter(component.getJsonValueFilter())
.map((jsonValue) -> jsonValue.key())
.forEach((key) -> component.add(key));
}
}
Quindi siamo passati da un'implementazione che supportava comportamenti molto specifici a uno in grado di gestire comportamenti arbitrari in un modo più generico, esponendo progressivamente le responsabilità. Non abbiamo programmato le responsabilità, però. Abbiamo appena seguito dove il codice e i refactoring ci hanno guidato. C'è ancora molto da fare con questo codice. Continuo a non pensare che le responsabilità siano distribuite correttamente. Ci sono un certo numero di brutti nomi. C'erano refactoring alternativi che avrebbero potuto essere migliori. Ma abbiamo iniziato un percorso per rivelare quelle responsabilità senza doverle identificare o progettare in anticipo.