Os tipos de exceções reutilizados devem ser preferidos aos de uso único?

8

Digamos que eu tenho Doors que são gerenciados por a DoorService. Ele DoorServiceé responsável por abrir, fechar e trancar as portas armazenadas no banco de dados.

public interface DoorService
{
    void open(Door door) throws DoorLockedException, DoorAlreadyOpenedException;

    void close(Door door) throws DoorAlreadyClosedException;

    /**
     * Closes the door if open
     */
    void lock(Door door) throws DoorAlreadyLockedException;
}

Para o método lock, existe um DoorAlreadyLockedExceptione, para o método aberto, existe um DoorLockedException. Esta é uma opção, mas existem outras opções possíveis:

1) Use DoorLockedExceptionpara tudo, é um pouco estranho ao capturar a exceção em uma lock()chamada

try
{
    doorService.lock(myDoor);
}
catch(DoorLockedException ex) // door ALREADY locked
{
    //error handling...
}

2) Tenha os 2 tipos de exceções DoorLockedExceptioneDoorAlreadyLockedException

3) Tenha os 2 tipos de exceções, mas deixe DoorAlreadyLockedException extends DoorLockedException

Qual é a melhor solução e por quê?

Inverno
fonte
1
Como você define "melhor"? Em C #, nem verificamos exceções. Acreditamos que isso torna nosso desenvolvimento mais simples e mais confiável.
Robert Harvey
2
Quão diferentes são as resoluções para AlreadyLocked e Locked?
Whatsisname 17/07/19
@RobertHarvey Considero exceções parte da lógica do método. A melhor solução é aquela que é mais explícita e mais lógica. Diferentemente do C #, estamos promovendo a explicitação por conveniência, forçando a captura de casos excepcionais e garantindo que ninguém chame um método sem ter um entendimento completo do que está acontecendo.
Inverno
1
Esses métodos devem simplesmente retornar um código de status. Você pode informar seus usuários com base no código recebido e obter um aumento de desempenho gratuitamente (as exceções são caras, cerca de 1000 a 10000 vezes mais caras do que retornar um código de status).
Robert Harvey
1
@RobertHarvey "as exceções são caras, cerca de 1000 a 10000 vezes mais caras do que o retorno de um código de status" Eu fiz algumas análises sobre isso em Java e o custo das exceções costuma ser exagerado. Eles 'podem' ser caros se você tiver pilhas muito profundas. No IIRC, tive que aumentar significativamente o tamanho máximo da pilha para poder criar pilhas grandes o suficiente para tornar as exceções lentas o suficiente para me preocupar. Até mesmo porcos como o Spring não fazem isso. Desde que você não esteja jogando e pegando loops apertados, é uma otimização prematura para evitar exceções.
21418 JimmyJames

Respostas:

21

Sei que as pessoas fazem muito uso de exceções para controle de fluxo, mas esse não é o maior problema aqui. Eu vejo uma enorme violação de separação de consulta de comando . Mas mesmo isso não é o pior.

Não, o pior está aqui:

/**
 * Closes the door if open
 */

Por que diabos todo o resto explode se sua suposição sobre o estado das portas está errada, mas lock()apenas corrige isso para você? Esqueça que isso torna impossível trancar a porta, o que é absolutamente possível e ocasionalmente útil. O problema aqui é que você misturou duas filosofias diferentes para lidar com suposições incorretas. Isso é confuso. Não faça isso. Não no mesmo nível de abstração com o mesmo estilo de nomenclatura. Ow! Leve uma dessas idéias para fora. Os métodos de serviço na porta devem funcionar da mesma maneira.

Quanto à violação da Separação de Consulta de Comando, não devo tentar fechar uma porta para descobrir se está fechada ou aberta. Eu deveria ser capaz de perguntar. O serviço de porta não fornece uma maneira de fazer isso sem alterar possivelmente o estado da porta. Isso piora muito os comandos que também retornam valores (um mal-entendido comum sobre o que é o CQS). Aqui, comandos de alteração de estado são a única maneira de fazer consultas! Ow!

Quanto às exceções serem mais caras que os códigos de status, isso é conversa sobre otimização. Rápido o suficiente é rápido o suficiente. Não, o verdadeiro problema é que os humanos não esperam exceções para casos típicos. Você pode discutir sobre o que é típico, tudo que você gosta. Para mim, a grande questão é quão legível você está criando o código de uso.

ensureClosed(DoorService service, Door door){
  // Need door closed and unlocked. No idea of its state. What to do?
  try {
    service.open(door)
    service.close(door)
  } 
  catch( DoorLockedException e ){
    //Have no way to unlock the door so give up and die
    log(e);
    throw new NoOneGaveMeAKeyException(e);      
  }
  catch( DoorAlreadyOpenedException e ){
    try { 
      service.close(door);
    }
    catch( DoorAlreadyClosedException e ){
      //Some multithreaded goof has been messing with our door.
      //Oh well, this is what we wanted anyway.
      //Hope they didn't lock it.
    }
  }
}

Por favor, não me faça escrever um código como este. Por favor, nos dê isLocked()e isClosed()métodos. Com quem eu posso escrever meus próprios ensureClosed()e ensureUnlocked()métodos fáceis de ler. Aqueles que só são lançados se suas condições de postagem forem violadas. Prefiro apenas descobrir que você já escreveu e testou, é claro. Apenas não os misture com os que jogam quando não podem mudar de estado. No mínimo, dê a eles nomes distintos.

Faça o que fizer, por favor, não ligue para nada tryClose(). Esse é um nome terrível.

Quanto DoorLockedExceptionsozinho vs também ter DoorAlreadyLockedExceptionmal dizer isso: é tudo sobre o uso de código. Por favor, não projete serviços como esse sem escrever o código de uso e ver a bagunça que você está criando. Refatorar e redesenhar até que o código de uso seja pelo menos legível. De fato, considere escrever o código usando primeiro.

ensureClosed(DoorService service, Door door){
  if( !service.isClosed(door) ){
    try{
      service.close(door);
    }
    catch( DoorAlreadyClosedException e ){
      //Some multithreaded goof has been messing with our door.
      //Oh well, this is what we wanted anyway.
      //Hope they didn't lock it.
    }
  } else {
    //This is what you wanted, so quietly do nothing. 
    //Why are you even here? Who bothers to write empty else conditions?
  }
}

ensureUnlocked(DoorService service, Door door){
  if( service.islocked(door) ){
    throw new NoOneGaveMeAKeyException(); 
  }
}
candied_orange
fonte
O lock()método que está chamando close()era apenas eu sendo preguiçoso ao escrever a pergunta, mas obrigado por me informar disso! Não vou gostar disso ao escrever código real. No meu caso, os três métodos nunca são realmente chamados sucessivamente. É apenas: chame um, se não houver uma exceção boa, mostre uma caixa de diálogo traduzida e possivelmente faça logout (se InvalidTokenException). Eu tenho isLocked()e isOpen()métodos e eles são usados ​​no fluxo de controle do servidor, mas não devem ser usados ​​como uma maneira de verificar se uma caixa de diálogo de erro deve ser mostrada; esse é um trabalho para uma exceção.
Inverno
Para o seu último ponto, sim, o código de uso parece um pouco melhor, DoorAlreadyLockedExceptionmas cria classes gêmeas.
Inverno
2
Se por classes individuais quer dizer as exceções-se deixe-me mostrar-lhe meu uso favorito da herança: public class DoorAlreadyLockedException inherits DoorLockedException {}. Defina uma classe em uma linha apenas para lhe dar um bom nome. Escolha o que você herdou sabiamente. Prefiro herdar o mais alto possível, sem ser desnecessariamente redundante, para evitar uma longa cadeia de herança.
Candied_orange 17/07/2018
Estou com vontade de mudar o nome NoOneGaveMeAKeyde NoOneGaveMeAKeyExceptionapenas para ser pendente. Caso contrário, eu concordo com o ponto principal, que é o de que antes do processamento, precisamos saber o estado do objeto.
Walfrat 18/07
2
Advogado do diabo - apenas ter isOpen/ isClosednão é bom o suficiente. Pense no sistema de arquivos. Você pode verificar se o arquivo existe / não existe, mas isso ainda pode mudar quando você tentar lê-lo ou gravá-lo, e IOExceptionisso resultará. Talvez, nesse contexto, haja realmente a chance de o estado ser inválido quando você abre ou fecha a porta.
18718 Tom G
8

Fazer uma distinção DoorLockedExceptione DoorAlreadyLockedExceptionsugerir que diferentes tipos de exceção estão sendo usados ​​para controle de fluxo, uma prática que já é amplamente considerada um antipadrão.

Ter vários tipos de exceção para algo tão benigno é gratuito. A complexidade adicional não é compensada pelos benefícios adicionais.

Basta usar DoorLockedExceptionpara tudo.

Melhor ainda, simplesmente não faça nada. Se a porta já estiver trancada, ela ainda estará no estado correto quando o lock()método for concluído. Não crie exceções para condições que não sejam dignas de nota.

Se você ainda não se sentir confortável com isso, renomeie seu método ensureDoorLocked().

Robert Harvey
fonte
4
Por favor, não use em excesso o meme "exceções para controle de fluxo", se as duas situações, por qualquer motivo, exigirem cursos de ação dramaticamente diferentes a serem resolvidos; então, ter exceções diferentes faz sentido. Não está claro a partir do exemplo, se for esse o caso.
Whatsisname 17/07/19
1
@ whatsisname: Se é um meme que não tem base, poste sua própria resposta nesse sentido.
Robert Harvey
1
As exceções do @RobertHarvey são usadas para o "fluxo de controle" das caixas de diálogo e registro de erros. O aplicativo lógico principal obviamente não é executado pelas cláusulas catch. Eu não acho que fazer uma distinção DoorLockedExceptione DoorAlreadyLockedExceptionsugerir que é apenas ter um manipulador de erros muito mais significativo para o lock()método, ao custo de ter uma classe quase duplicada.
Inverno
1
Se você chamar lock () e a porta estiver trancada, você já sabe que a porta já estava trancada. Você não precisa saber o que você já sabe.
Robert Harvey
2
@ewan: Oh, ffs. Chame como quiser; uma enumeração, um resultado, qualquer que seja. É a abordagem correta.
Robert Harvey
3

O assunto desta pergunta "Os tipos de exceções recicladas devem ser favorecidos em relação aos de uso único?" parece ter sido ignorado, não apenas nas respostas, mas nos detalhes das perguntas (as respostas foram nos detalhes da pergunta).

Concordo com a maioria das críticas ao código que os entrevistados ofereceram.

Mas, como resposta à pergunta principal, eu diria que você deve preferir reutilizar 'exceções recicladas' em vez de 'uso único'.

No uso mais típico do tratamento de exceções, você tem uma grande DISTÂNCIA no código entre o local onde a exceção é detectada e lançada e onde a exceção é capturada e manipulada. Isso significa que o uso de tipos privados (modulares) no tratamento de exceções geralmente não funcionará bem. Um programa típico com manipulação de exceções - terá vários locais de nível superior no código em que as exceções são tratadas. E como esses são locais de nível superior, é difícil para eles terem conhecimento detalhado sobre aspectos estreitos de módulos específicos.

Em vez disso, eles provavelmente terão manipuladores de alto nível para alguns problemas de alto nível.

A única razão pela qual classes de exceção personalizadas detalhadas parece fazer sentido em sua amostra é porque (e aqui estou repetindo os outros entrevistados) - você não deve usar o tratamento de exceções para o fluxo de controle comum.

Lewis Pringle
fonte
1
"No uso mais típico do tratamento de exceções, você tem uma grande DISTÂNCIA" Não estamos analisando o que é típico. Estamos projetando uma maneira de funcionar. Muitos bons designs capturam suas exceções no próximo nível na pilha de chamadas e em um escopo extremamente limitado. Isso não deve ser descartado como controle de fluxo quando tudo o que eles fazem é se recuperar do problema. Os melhores designs de exceção começam planejando como se recuperar antes de definir as exceções. Os nomes das exceções não precisam ser sobre como reportar o problema. Eles são sobre encontrar a solução. Você pode usar a string msg para relatar o problema.
Candied_orange 17/07/2018
"Não estamos analisando o que é típico. Estamos criando uma maneira de funcionar.", Concordou, era o que você estava fazendo. Não é o que eu estava fazendo. Se você olhar para o título, ele fez uma pergunta. Se você olhar o corpo da mensagem, ela fará três perguntas diferentes (relacionadas). Você respondeu às perguntas do corpo (até certo ponto - na verdade, mais você criticou o código de exemplo). Eu respondi à pergunta principal.
21718 Lewis Pringle
BTW, você afirma "Muitos bons designs capturam suas exceções no próximo nível na pilha de chamadas e em um escopo extremamente limitado". Não tenho certeza se discordo totalmente disso. Mas tenho que dizer que, depois de 40 anos de programação, estou tendo dificuldades para encontrar um exemplo para apoiar essa afirmação. Eu percebo que é um pouco complicado (e talvez eu não esteja lidando com isso corretamente - novo no stackoverflow) -, mas você gostaria de oferecer um exemplo ou dois em que esse tratamento localizado de tentativa / captura é a melhor solução? Certamente não é um padrão típico / comum.
Lewis Pringle
Um exemplo hein? Deixe-me apresentá-lo a Jon Skeet #
candied_orange
OK, isso não é um exemplo do que você reivindicou. Você disse: "Muitos projetos bons capturam suas exceções no próximo nível na pilha de chamadas e em um escopo extremamente limitado". Seu exemplo foi apenas a tradução de uma API que produziu apenas exceções por erro em uma API que produziu algum valor de sentinela. Talvez seja apenas uma diferença de expressão entre nós dois. Eu não descreveria isso como um "bom design", mas um 'hack localizado para contornar uma opção de design inconveniente em uma ferramenta que você está usando'.
Lewis Pringle
0

Uma alternativa inexplorada. Você pode estar modelando a coisa errada com suas aulas, e se você as tivesse?

public interface OpenDoor
{
    public ClosedDoor close();
    public LockedDoor lock();
}

public interface ClosedDoor
{
    public OpenDoor open();
    public LockedDoor lock();
}

public interface LockedDoor
{
    // ? unlock? open?
}

Agora é impossível tentar qualquer coisa que seja um erro. Você não precisa de nenhuma exceção.

Caleth
fonte
Mas na maioria dos aplicativos, você não pode manter a referência a esses objetos, pois o banco de dados pode ser alterado a qualquer momento. No entanto, um bom esforço criativo
Inverno
0

Responderei à pergunta original, em vez de criticar o exemplo.

Simplificando, se você espera que o cliente precise reagir de maneira diferente para cada caso, ele garante um novo tipo de exceção.

Se você decidir usar vários tipos de exceção e, no entanto, esperar que, em alguns casos, o cliente queira criar uma cláusula de captura comum para eles, provavelmente crie uma superclasse abstrata que os dois estendem.

Gudmundur Orn
fonte
0

Este é um pseudocódigo, mas acho que você entenderá o que quero dizer:

public interface DoorService
{
    // ensures the door is open, uses key if locked, returns false if lock without a key or key does not fit
    bool Open(Door door, optional Key key) throws DoorStuckException, HouseOnFireException;

    // closes the door if open. already closed doors stay closed. Locked status does not change.
    void Close(Door door) throws throws DoorStuckException, HouseOnFireException;

    // closes the door if open and locks it
    void CloseAndLock(Door door, Key key) throws DoorStuckException, HouseOnFireException;
}

Sim, a reutilização do tipo de exceção faz sentido, se você usar exceções para coisas realmente excepcionais . Porque coisas verdadeiramente excepcionais são principalmente preocupações transversais. Você estava usando Exceções basicamente para controlar fluxos e isso leva a esses problemas.

nvoigt
fonte
Mas esses tipos de exceção são realmente não descritivos. O que significa ter uma porta "presa" quando você só pode abrir, fechar ou trancar uma porta?
Inverno
Isso é apenas engolir as exceções e não faz nada com elas. Isso não é apropriado em um contexto em que você não quer saber o que está acontecendo. Parece a maneira C # do meu exemplo de Java. Há uma razão pela qual estou trabalhando com Java.
1919 Winter
Bem, isso está preso. Não pode se mover. Então você não pode abri-lo e não pode fechá-lo. Não é um estado lógico que você precisa corrigir chamando outro método, é um estado excepcional que você não pode resolver através do serviço de porta.
Nvoigt 19/07/19
E não está engolindo nada , não está produzindo uma exceção para um estado de programa não excepcional. Se minha esposa me pedir para fechar a porta da varanda e eu me levantar e descobrir que ela já está fechada, não corro por aí, grito e entrei em pânico. Eu apenas volto e digo "está fechado agora".
Nvoigt 19/07/19
Exatamente, com exceções verificadas, a exceção que não pode voltar ao início do programa deve ser capturada pelo chamador. Na chamada, você simplesmente ignora a exceção ou registra um aviso e está bem. Além disso, o que você chama de "exceções excepcionais" são apenas RuntimeExceptions. DoorStuckExceptione HouseOnFireExceptionnão devem ser verificados, eles não devem ser capturados em todas as chamadas.
19418 Winter