O uso da cláusula finalmente para fazer o trabalho após o retorno é ruim / perigoso?

30

Como parte de escrever um Iterator, eu me vi escrevendo o seguinte trecho de código (removendo o tratamento de erros)

public T next() {
  try {
    return next;
  } finally {
    next = fetcher.fetchNext(next);
  }
}

achando um pouco mais fácil de ler do que

public T next() {
  T tmp = next;
  next = fetcher.fetchNext(next);
  return tmp;
}

Sei que é um exemplo simples, onde a diferença de legibilidade pode não ser tão impressionante, mas estou interessado na opinião geral sobre se é ruim usar o try-finalmente em casos como esse, onde não há exceções envolvidas, ou se é realmente preferido quando simplifica o código.

Se é ruim: por quê? Estilo, desempenho, armadilhas, ...?

Conclusão Obrigado por todas as suas respostas! Acho que a conclusão (pelo menos para mim) é que o primeiro exemplo pode ter sido mais legível se fosse um padrão comum, mas não é. Portanto, a confusão introduzida usando uma construção fora do seu objetivo, juntamente com o fluxo de exceção possivelmente ofuscado, superará quaisquer simplificações.

Halle Knast
fonte
10
Pessoalmente, eu seria capaz de entender o segundo mais do que o primeiro, mas raramente uso finallyblocos.
Christian Mann
2
retornar atual = fetcher.fetchNext (atual); // Que tal isso, aka, você realmente precisa de pré-busca?
scarfridge
11
@scarfridge O exemplo é sobre a implementação Iterator, onde você realmente precisa de uma pré-busca para hasNext()poder trabalhar. Tente você mesmo.
Maaartinus
11
@maaartinus Estou ciente disso. Às vezes, hasNext () simplesmente retorna true, por exemplo, seqüência de granizo e, às vezes, buscar o próximo elemento é proibitivamente caro. Neste último caso, você deve buscar o elemento apenas sob demanda, semelhante à inicialização lenta.
scarfridge
11
@scarfridge O problema com o uso comum de Iteratoré que você precisa buscar o valor hasNext()(porque buscá- lo é muitas vezes a única maneira de descobrir se existe) e devolvê-lo next()exatamente como o OP fez.
Maaartinus 28/04

Respostas:

18

Pessoalmente, eu pensaria na idéia de separação de consulta de comando . Quando você vai direto ao assunto, next () tem dois propósitos: o objetivo anunciado de recuperar o elemento next e o efeito colateral oculto de alterar o estado interno do next. Então, o que você está fazendo é realizar o objetivo anunciado no corpo do método e, em seguida, aplicar um efeito colateral oculto em uma cláusula finalmente, o que parece ... estranho, embora não seja "errado", exatamente.

O que realmente se resume a quão compreensível é o código, eu diria, e nesse caso a resposta é "meh". Você está "tentando" uma declaração de retorno simples e, em seguida, executa um efeito colateral oculto em um bloco de código que deveria ser usado para recuperação de erros à prova de falhas. O que você está fazendo é 'inteligente', e o código 'inteligente' muitas vezes induz os mantenedores a murmurar: "o que ... oh, acho que entendi". Melhor para as pessoas que leem seu código murmurar "sim, uh-huh, faz sentido, sim ..."

Então, e se você separasse a mutação de estado das chamadas de acessador? Imagino que o problema de legibilidade que você está preocupado se torne discutível, mas não sei como isso afeta sua abstração mais ampla. Algo a considerar, de qualquer maneira.

Erik Dietrich
fonte
11
+1 no comentário "inteligente". Isso captura com muita precisão este exemplo.
2
A ação 'return and advance' de next () é imposta ao OP pela interface do iterador Java pesada.
Kevin cline
37

Puramente do ponto de vista do estilo, acho que essas três linhas:

T current = next;
next = fetcher.getNext();
return current;

... são mais óbvios e mais curtos que um bloco try / finalmente. Como você não espera que nenhuma exceção seja lançada, o uso de um trybloco só vai confundir as pessoas.

StriplingWarrior
fonte
2
Um bloco try / catch / finalmente também pode adicionar uma sobrecarga extra; não tenho certeza se o javac ou o compilador JIT os removeria mesmo que você não esteja lançando uma exceção.
Martijn Verburg
2
@MartijnVerburg sim, o javac ainda adiciona uma entrada à tabela de exceções e duplica o código finalmente, mesmo que o bloco try esteja vazio.
Daniel Lubarov
@ Daniel - graças eu era muito preguiçoso para executar javap :-)
Martijn Verburg
@Martijn Verburg: AFAIK, um bloco try-catch-finallt é essencialmente gratuito, desde que não exista nenhuma exceção real. Aqui, o javac não tenta e não precisa otimizar nada, pois o JIT cuidará disso.
Maaartinus
@maaartinus - graças que faz sentido - necessidade de ler o código-fonte OpenJDK novamente :-)
Martijn Verburg
6

Isso dependeria da finalidade do código dentro do bloco final. O exemplo canônico é fechar um fluxo depois de ler / escrever nele, algum tipo de "limpeza" que sempre deve ser feita. Restaurando um objeto (neste caso, um iterador) para um estado válido IMHO também conta como limpeza, então não vejo problema aqui. Se você usasse o OTOH returnassim que seu valor de retorno fosse encontrado e adicionasse muito código não relacionado no bloco final, isso obscureceria o objetivo e tornaria tudo menos compreensível.

Não vejo problema em usá-lo quando "não há exceções envolvidas". É muito comum usar try...finallysem um catchquando o código pode ser lançado apenas RuntimeExceptione você não planeja manipulá-lo. Às vezes, o finalmente é apenas uma salvaguarda, e você sabe pela lógica do seu programa que nenhuma exceção será lançada (a condição clássica "isso nunca deve acontecer").

Armadilhas: qualquer exceção levantada dentro do trybloco fará o finallybock correr. Isso pode colocá-lo em um estado inconsistente. Portanto, se sua declaração de retorno fosse algo como:

return preProcess(next);

e esse código gerou uma exceção, fetchNextainda seria executado. OTOH se você o codificou como:

T ret = preProcess(next);
next = fetcher.fetchNext(next);
return ret;

então não funcionaria. Sei que você está assumindo que o código try nunca pode gerar nenhuma exceção, mas em casos mais complexos que isso, como você pode ter certeza? Se o seu iterador fosse um objeto de longa duração, ele continuaria existindo mesmo se um erro irrecuperável ocorresse no encadeamento atual, seria importante mantê-lo em um estado válido o tempo todo. Caso contrário, isso realmente não importa muito ...

Desempenho: seria interessante descompilar esse código para ver como ele funciona, mas eu não conheço o suficiente da JVM para adivinhar ... As exceções são geralmente "excepcionais", portanto, o código que lida com eles não precisam ser otimizados para velocidade (daí o conselho para nunca usar exceções no fluxo de controle normal de seus programas), mas não sei finally.

mgibsonbr
fonte
Você não está realmente fornecendo um bom motivo para evitar usá finally-lo dessa maneira? Quero dizer, como você diz, o código acaba tendo consequências indesejadas; Pior ainda, você provavelmente nem está pensando em exceções.
Andres F.
2
A velocidade normal do fluxo de controle não é afetada significativamente pelas construções de tratamento de exceções. A penalidade de desempenho é paga apenas ao realmente lançar e capturar exceções, não ao inserir um bloco de tentativa ou finalmente um bloco em operação normal.
precisa saber é o seguinte
11
@AndresF. Verdade, mas isso também é válido para qualquer código de limpeza. Por exemplo, suponha que uma referência a um fluxo aberto seja definida nullpor um código de buggy; tentar ler dele gerará uma exceção, tentar fechá-lo no finallybloco gerará outra exceção. Além disso, esta é uma situação em que o estado da computação não importa, você deseja fechar o fluxo, independentemente de uma exceção. Pode haver outras situações como essa também. Por esse motivo, estou tratando-o como uma armadilha para os usos válidos, em vez de uma recomendação para nunca usá-lo.
mgibsonbr
Também existe o problema de que, quando a tentativa e finalmente o bloco lançam uma exceção, a exceção na tentativa nunca é vista por ninguém, mas provavelmente é a causa do problema.
Hans-Peter Störr
3

A declaração do título: "... finalmente cláusula para realizar o trabalho após o retorno ..." é falsa. O bloco finalmente acontece antes que a função retorne. Esse é o objetivo de finalmente, de fato.

O que você está abusando aqui é a ordem da avaliação, na qual o valor de next é armazenado para retornar antes de você o alterar. Não é uma prática comum e, na minha opinião, errada, pois faz com que você codifique não sequencial e, portanto, muito mais difícil de seguir.

O uso pretendido dos blocos finalmente é para tratamento de exceções, onde algumas conseqüências devem ocorrer, independentemente de exceções serem lançadas, por exemplo, limpar algum recurso, fechar a conexão com o banco de dados, fechar um arquivo / soquete etc.

Nitsan Wakart
fonte
+1, eu acrescentaria que, mesmo que a especificação do idioma garanta que o valor seja armazenado antes de ser retornado, ele contraria as expectativas do leitor e, portanto, deve ser evitado quando razoavelmente possível. A depuração é duas vezes tão duro como codificação ...
jmoreno
0

Eu ficaria muito cuidadoso, porque (em geral, não no seu exemplo) parte da tentativa pode gerar uma exceção e, se for descartada, nada será retornado e se você realmente pretender executar o que está finalmente após o retorno, será executar quando você não queria.

E outra lata de worm é que, em geral, alguma ação útil finalmente pode gerar exceções, para que você possa terminar com um método realmente confuso.

Por causa disso, alguém que o lê deve pensar nesses problemas, por isso é mais difícil de entender.

user470365
fonte