Leggibilità: meglio fuori che dentro

Un argomento di cui recentemente si discute non poco nel nostro team è l'argomento leggibilità.
Premesso che la leggibilità è sempre un soggetto altamente personale, ciò che per me può essere perfettamente leggibile può non esserlo per altri, la mia opinione è che, dovendo scegliere, è sempre meglio preferire la leggibilità esterna di una classe, piuttosto che quella interna.

"Meglio fuori che dentro, dico sempre io"

Difatti, in teoria (specie se si vuole rispettare OCP), una classe dovrebbe essere modificata il meno possibile, e se scriviamo del buon design ben orientato agli oggetti è probabile che la classe stessa non sarà modificata mai, ma verranno solo aggiunte nuove implementazioni di interfacce a fronte di nuove funzionalità.
La leggibilità interna ha quindi un valore abbastanza relativo, basta cioè che sia sufficientemente leggibile da rispettare principi come avere metodi corti e ben definiti, pochi (meglio niente) cicli for e if statement innestati in cascata (ma piuttosto estratti in metodi),  nessuna ambiguità su ingressi e uscite, meglio ancora se con un forte controllo sui tipi (per quanto in java i generics siano molto verbosi, preferisco usarli sempre e comunque quando servono, perchè offrono forti garanzie sulla correttezza dei tipi in uso).

Un esempio concreto: la classe sottostante effettivamente ha una leggibilità interna sufficiente, ma non esaltante.
Le classi inner e l'uso pesante di reflection (purtroppo inevitabile, dato lo scopo di questa classe) sicuramente confondono molto, e anche la gestione delle eccezioni ci mette il suo zampino.
E' anche vero però che la classe principale ha un costruttore privato, e l'unico modo di istanziarla è usando l'unico metodo pubblico disponibile: il metodo "search(enumClass)". Da lì, navigando il codice seguendo lo stack delle chiamate, è possibile seguire il flusso senza perdersi troppo, dato che le biforcazioni non sono molte.
Inoltre i metodi (così come le inner class) sono abbastanza compatti e focalizzati.
Si riesce a capire cosa fa questa classe? (in basso la risposta...)

package com.gmail.gulino.marco.utils;

import static com.google.common.collect.Iterables.find;
import static com.google.common.collect.Lists.newArrayList;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.List;

import com.google.common.base.Predicate;

@SuppressWarnings("unchecked")
public class SmartEnums<EnumType extends Enum<EnumType>> {

        public static <EnumType extends Enum<EnumType>> SmartEnums<EnumType> search(Class<EnumType> enumClass) {
                return new SmartEnums<EnumType>(enumClass);
        }

        private List<EnumType> allValues;

        private SmartEnums(Class<EnumType> enumClass) {
                try {
                        Method valuesMethod = enumClass.getMethod("values");
                        this.allValues = newArrayList((EnumType[]) valuesMethod.invoke(enumClass));
                } catch (Exception e) {
                        throw new RuntimeException("Are you trying to apply SmartEnums to a non enum class?");
                }
        }

        public MemberSearch byField(final String fieldName) {
                return new MemberSearch() {

                        @Override
                        protected Object findValueOn(EnumType enumType) throws Exception {
                                Field field = enumType.getClass().getDeclaredField(fieldName);
                                field.setAccessible(true);
                                return field.get(enumType);
                        }
                };
        }

        public MemberSearch byMethod(final String methodName) {
                return new MemberSearch() {

                        protected Object findValueOn(EnumType enumType) throws Exception {
                                Method method = enumType.getClass().getDeclaredMethod(methodName);
                                method.setAccessible(true);
                                return method.invoke(enumType);
                        }

                };
        }

        public abstract class MemberSearch {

                public EnumType havingValue(Object value) {
                        try {
                                return find(allValues, valueFinderPredicate(value));
                        } catch (Exception e) {
                                throw new RuntimeException(e);
                        }
                }

                protected abstract Object findValueOn(EnumType enumType) throws Exception;

                protected Predicate<EnumType> valueFinderPredicate(final Object value) {
                        return new Predicate<EnumType>() {
                                @Override
                                public boolean apply(EnumType enumType) {
                                        try {
                                                return value.equals(findValueOn(enumType));
                                        } catch (Exception e) {
                                                throw new RuntimeException(e);
                                        }
                                }
                        };
                }

        }
}

Certo, come ho già scritto prima la leggibilità interna non è esaltante...
Ma vediamo come viene utilizzata esternamente questa classe, nella fattispecie il suo unit test::

package com.gmail.gulino.marco.utils;

import static com.gmail.gulino.marco.utils.SmartEnums.search;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.Assert.assertThat;

import org.junit.Test;

public class SmartEnumsTest {
        @Test
        public void itShouldRetrieveEnumByField() throws Exception {
                assertThat(search(AnExampleEnum.class).byField("enumFieldToSearchFor").havingValue("first"), equalTo(AnExampleEnum.firstElement));
                assertThat(search(AnExampleEnum.class).byField("enumFieldToSearchFor").havingValue("second"), equalTo(AnExampleEnum.secondElement));
        }
        @Test
        public void itShouldRetrieveEnumByMethods() throws Exception {
                assertThat(search(AnExampleEnum.class).byMethod("enumMethodToSearchFor").havingValue("first"), equalTo(AnExampleEnum.firstElement));
                assertThat(search(AnExampleEnum.class).byMethod("enumMethodToSearchFor").havingValue("second"), equalTo(AnExampleEnum.secondElement));
        }

        public enum AnExampleEnum {
                firstElement("first"), secondElement("second");
                private final String enumFieldToSearchFor;

                private AnExampleEnum(String value) {
                        this.enumFieldToSearchFor = value;
                }

                public String enumMethodToSearchFor() {
                        return enumFieldToSearchFor;
                }

        }
}

Le assert sono chiare: traducendo letteralmente dall'inglese, abbiamo, mettendo tra parentesi le parole sottointese:

Asserisci (che la) ricerca (di) AnExampleEnum per field "enumFieldToSearchFor" avente valore "first" (sia) uguale a AnExampleEnum.firstElement

Le parole sottointese sono ben poche, è quindi evidente che fa la classe:

Ricerca una enum per field "nome del field" avente valore "valore da ricercare".

o, nella seconda delle due interfacce disponibili:

Ricerca una enum per metodo "nome del metodo" avente valore "valore da ricercare".

Ora, il vantaggio evidente è che gli utilizzatori di questa classe non dovranno scervellarsi troppo nel capirla, in effetti non avranno proprio alcun bisogno di aprire l'implementazione e leggerla. Basta solo leggere il suo test (o un eventuale altro utilizzo) per capire cosa fa e come va invocata.