alternativas a las trampas anidadas para retrocesos

14

Tengo una situación en la que intento recuperar un objeto. Si la búsqueda falla, tengo varios fallos en su lugar, cada uno de los cuales puede fallar. Entonces el código se ve así:

try {
    return repository.getElement(x);
} catch (NotFoundException e) {
    try {
        return repository.getSimilarElement(x);
    } catch (NotFoundException e1) {
        try {
            return repository.getParentElement(x);
        } catch (NotFoundException e2) {
            //can't recover
            throw new IllegalArgumentException(e);
        }
    }
}

Esto se ve terriblemente feo. Odio volver nulo, pero ¿es eso mejor en esta situación?

Element e = return repository.getElement(x);
if (e == null) {
    e = repository.getSimilarElement(x);
}
if (e == null) {
    e = repository.getParentElement(x);
}
if (e == null) {
    throw new IllegalArgumentException();
}
return e;

¿Hay otras alternativas?

¿Usar bloques anidados try-catch es un antipatrón? está relacionado, pero las respuestas allí están en la línea de "a veces, pero generalmente es evitable", sin decir cuándo o cómo evitarlo.

Alex Wittig
fuente
1
¿Es NotFoundExceptionalgo realmente excepcional?
No lo sé, y probablemente sea por eso que estoy teniendo problemas. Esto es en un contexto de comercio electrónico, donde los productos se suspenden diariamente. Si alguien marca un producto que se descontinúa posteriormente, y luego intenta abrir el marcador ... ¿eso es excepcional?
Alex Wittig
@FiveNine en mi opinión, definitivamente no, es de esperar. Ver stackoverflow.com/questions/729379/…
Konrad Morawski

Respuestas:

17

La forma habitual de eliminar el anidamiento es usar funciones:

Element getElement(x) {
    try {
        return repository.getElement(x);
    } catch (NotFoundException e) {
        return fallbackToSimilar(x);
    }  
}

Element fallbackToSimilar(x) {
    try {
        return repository.getSimilarElement(x);
     } catch (NotFoundException e1) {
        return fallbackToParent(x);
     }
}

Element fallbackToParent(x) {
    try {
        return repository.getParentElement(x);
    } catch (NotFoundException e2) {
        throw new IllegalArgumentException(e);
    }
}

Si estas reglas de respaldo son universales, podría considerar implementar esto directamente en el repositoryobjeto, donde podría usar simplemente ifdeclaraciones simples en lugar de una excepción.

Karl Bielefeldt
fuente
1
En este contexto, methodsería una palabra mejor que function.
Sulthan
12

Esto sería realmente fácil con algo como una mónada Option. Desafortunadamente, Java no tiene esos. En Scala, usaría el Trytipo para encontrar la primera solución exitosa.

En mi mentalidad de programación funcional, configuré una lista de devoluciones de llamada que representan las diversas fuentes posibles, y las recorrí hasta encontrar la primera exitosa:

interface ElementSource {
    public Element get();
}

...

final repository = ...;

// this could be simplified a lot using Java 8's lambdas
List<ElementSource> sources = Arrays.asList(
    new ElementSource() {
        @Override
        public Element get() { return repository.getElement(); }
    },
    new ElementSource() {
        @Override
        public Element get() { return repository.getSimilarElement(); }
    },
    new ElementSource() {
        @Override
        public Element get() { return repository.getParentElement(); }
    }
);

Throwable exception = new NoSuchElementException("no sources set up");
for (ElementSource source : sources) {
    try {
        return source.get();
    } catch (NotFoundException e) {
        exception = e;
    }
}
// we end up here if we didn't already return
// so throw the last exception
throw exception;

Esto se puede recomendar solo si realmente tiene una gran cantidad de fuentes, o si tiene que configurar las fuentes en tiempo de ejecución. De lo contrario, esta es una abstracción innecesaria y se beneficiaría más de mantener su código simple y estúpido, y simplemente usar esos feos trucos anidados.

amon
fuente
+1 por mencionar el Trytipo en Scala, por mencionar mónadas y por la solución usando un bucle.
Giorgio
Si ya estuviera en Java 8, iría por esto, pero como usted dice, es un poco demasiado solo por unos pocos fallos.
Alex Wittig
1
En realidad, cuando se publicó esta respuesta, Java 8 con soporte para la Optionalmónada ( prueba ) ya se había lanzado.
mkalkov
3

Si está anticipando que se lanzarán muchas de esas llamadas al repositorio NotFoundException, puede usar un contenedor alrededor del repositorio para simplificar el código. No recomendaría esto para operaciones normales, ten en cuenta:

public class TolerantRepository implements SomeKindOfRepositoryInterfaceHopefully {

    private Repository repo;

    public TolerantRepository( Repository r ) {
        this.repo = r;
    }

    public SomeType getElement( SomeType x ) {
        try {
            return this.repo.getElement(x);
        }
        catch (NotFoundException e) {
            /* For example */
            return null;
        }
    }

    // and the same for other methods...

}
Rory Hunter
fuente
3

A sugerencia de @ amon, aquí hay una respuesta que es más monádica. Es una versión muy resumida, donde debes aceptar algunas suposiciones:

  • la función "unidad" o "retorno" es el constructor de la clase

  • la operación de "vinculación" ocurre en tiempo de compilación, por lo que está oculta a la invocación

  • las funciones de "acción" también están vinculadas a la clase en tiempo de compilación

  • Aunque la clase es genérica y envuelve cualquier clase arbitraria E, creo que en realidad es exagerado en este caso. Pero lo dejé así como un ejemplo de lo que podrías hacer.

Con esas consideraciones, la mónada se traduce en una clase de envoltura fluida (aunque está renunciando a la flexibilidad que obtendría en un lenguaje puramente funcional):

public class RepositoryLookup<E> {
    private String source;
    private E answer;
    private Exception exception;

    public RepositoryLookup<E>(String source) {
        this.source = source;
    }

    public RepositoryLookup<E> fetchElement() {
        if (answer != null) return this;
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookup(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public RepositoryLookup<E> orFetchSimilarElement() {
        if (answer != null) return this; 
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookupVariation(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public RepositoryLookup<E> orFetchParentElement() {
        if (answer != null) return this; 
        if (! exception instanceOf NotFoundException) return this;

        try {
            answer = lookupParent(source);
        }
        catch (Exception e) {
            exception = e;
        }

        return this;
    }

    public boolean failed() {
        return exception != null;
    }

    public Exception getException() {
        return exception;
    }

    public E getAnswer() {
        // better to check failed() explicitly ;)
        if (this.exception != null) {
            throw new IllegalArgumentException(exception);
        }
        // TODO: add a null check here?
        return answer;
    }
}

(esto no se compilará ... ciertos detalles se dejan sin terminar para mantener la muestra pequeña)

Y la invocación se vería así:

Repository<String> repository = new Repository<String>(x);
repository.fetchElement().orFetchParentElement().orFetchSimilarElement();

if (repository.failed()) {
    throw new IllegalArgumentException(repository.getException());
}

System.err.println("Got " + repository.getAnswer());

Tenga en cuenta que tiene la flexibilidad para componer las operaciones de "búsqueda" como desee. Se detendrá cuando reciba una respuesta o una excepción que no sea encontrada.

Hice esto muy rápido; no está del todo bien, pero con suerte transmite la idea

Robar
fuente
1
repository.fetchElement().fetchParentElement().fetchSimilarElement();- en mi opinión: código malvado (en el sentido dado por Jon Skeet)
Konrad Morawski
A algunas personas no les gusta ese estilo, pero el uso return thispara crear llamadas de objetos en cadena ha existido durante mucho tiempo. Como OO involucra objetos mutables, return thises más o menos equivalente a return nullsin encadenar. Sin embargo, return new Thing<E>abre la puerta a otra capacidad en la que este ejemplo no entra, por lo que es importante para este patrón si elige seguir este camino.
Rob
1
Pero me gusta ese estilo y no estoy en contra de encadenar llamadas o interfaces fluidas como tales. Sin embargo, existe una diferencia entre CustomerBuilder.withName("Steve").withID(403)este código, y solo por verlo .fetchElement().fetchParentElement().fetchSimilarElement()no está claro qué sucede, y esa es la clave aquí. ¿Todos van a buscarlos? No es acumulativo en este caso, y por lo tanto no es tan intuitivo. Necesito ver eso if (answer != null) return thisantes de que realmente lo entienda. Quizás es solo una cuestión de nombrar correctamente ( orFetchParent), pero de todos modos es "mágico".
Konrad Morawski
1
Por cierto (sé su código se simplifica y sólo una prueba de concepto), que sería bueno para volver quizá un clon de answeren getAnswery de reposición (borrar) el answercampo mismo, antes de regresar su valor. De lo contrario, se rompe el principio de separación de comando / consulta, porque pedir recuperar un elemento (consulta) altera el estado de su objeto de depósito ( answernunca se restablece) y afecta el comportamiento de fetchElementcuando lo llame la próxima vez. Sí, estoy un poco quisquilloso, creo que la respuesta es válida, no fui yo quien la rechazó.
Konrad Morawski
1
ese es un buen punto. Otra forma sería "intentToFetch ...". El punto importante es que en este caso, se llaman los 3 métodos, pero en otro caso, un cliente podría usar "intentFetch (). IntentFetchParent ()". Además, llamarlo "Repositorio" está mal, porque realmente está modelando una sola búsqueda. Tal vez me preocupe con los nombres para llamarlo "RepositoryLookup" e "intento" para dejar en claro que este es un artefacto transitorio de una sola vez que proporciona cierta semántica en torno a una búsqueda.
Rob
2

Otra forma de estructurar una serie de condiciones como esta es llevar una bandera, o probar nulo (mejor aún, use Guava's Opcional para determinar cuándo está presente una buena respuesta) para encadenar las condiciones.

Element e = null;

try {
    e = repository.getElement(x);
} catch (NotFoundException e) {
    // nope -- try again!
}

if (e == null) {  // or ! optionalElement.isPresent()
    try {
        return repository.getSimilarElement(x);
    } catch (NotFoundException e1) {
        // nope -- try again!
    }
}

if (e == null) {  // or ! optionalElement.isPresent()
    try {
        return repository.getParentElement(x);
    } catch (NotFoundException e2) {
        // nope -- try again!
    }
}

if (e == null) {  // or ! optionalElement.isPresent()
    //can't recover
    throw new IllegalArgumentException(e);
}

return e;

De esa forma, observa el estado del elemento y realiza las llamadas correctas en función de su estado, es decir, siempre y cuando aún no tenga una respuesta.

(Sin embargo, estoy de acuerdo con @amon. Recomiendo mirar un patrón Monad, con un objeto de envoltura como class Repository<E>ese tiene miembros E answer;y Exception error;. En cada etapa verifique si hay una excepción, y si es así, omita cada paso restante. Al final, te queda una respuesta, la ausencia de una respuesta o una excepción y puedes decidir qué hacer con eso).

Robar
fuente
-2

Primero, me parece que debería haber una función como repository.getMostSimilar(x)(debería elegir un nombre más apropiado) ya que parece haber una lógica que se utiliza para encontrar el elemento más cercano o más similar para un elemento dado.

El repositorio puede implementar la lógica como se muestra en amons post. Eso significa que el único caso en que se debe lanzar una excepción es cuando no se puede encontrar un elemento único.

Sin embargo, esto solo es posible si las lógicas para encontrar el elemento más cercano se pueden encapsular en el repositorio. Si esto no es posible, proporcione más información sobre cómo (según qué criterios) se puede elegir el elemento más cercano.

valenterry
fuente
las respuestas son para responder a la pregunta, no para solicitar aclaraciones
mosquito
Bueno, mi respuesta es resolver su problema, ya que muestra una manera de evitar los intentos / capturas anidados bajo ciertas condiciones. Solo si no se cumplen estas condiciones, necesitamos más información. ¿Por qué esta no es una respuesta válida?
valenterry