Is using an empty catch a bad practice?

Asked

Viewed 990 times

17

There have been cases where I needed to make use of methods that launched exception if any error occurred, but the exception would stop the execution of script, what would be something unwanted.

For example, I have a system where, when the student registers the request, an email is sent. The function that sends the email throws the exception. However, as the exception would stop the registration, I preferred to use a catch empty. It was not as important to send the email as the request was registered.

So I did something like this:

try {

    Mail::create('email.aluno')->send($callback);

} catch (\Exception $e) {
    // Se não enviar o e-mail por conta de um erro
    // continue o script
    // Não vamos interromper esse momento tão importante
}

$usuario->fill($dados)->save();

One could say: just take the exception from where it is thrown. I don’t do this, because changing the third-party source code is not a good idea, because when updating the system, I would have to edit again, and again and again...

It’s a bad practice to use a catch empty for nothing to happen when the exception occurs?

That would be an anti-standard?

There is a specific name for this type of operation?

There are risks in doing such an operation?

  • I think this answer shows that it is bad practice to leave the catch empty :) : http://answall.com/a/23787/27190

  • 4

    ugly as hell..... you can solve this with an if instead of using Try. is a system that does not treat exceptions, is already something bad, empty catch so...

  • @Rboschini but with if, how would you "stop" the exception release?

  • exceptions are predictable, I would do the tests, identify the exceptions and deal with IF if necessary, never with an empty Try.

  • 7

    I find the question complicated without specifying the language. For example, in Java, roughly speaking, everything is an exception. In PHP, almost (practically) everything is solved without exception. And, as the name says, exception should be treated as an exception (but in Java, make an exception to what I said). :)

  • A programming bug has to be predictable, if you do not understand why the exception is released or when it occurs, it is because you have not done the necessary tests to identify, you use Try to pass something you can treat the right way without bursting in the face of the user.

  • @Rboschini maybe then I can take the specific exception and ignore it or else not ignore it. For example, the question of catch multiple.

  • Multiple catch this OK, empty catch that no, in my opinion. ignore an exception I see how to prevent it from occurring, never ignore exception, give me an example of exceptions that you could ignore.

  • If Try::catch is mandatory, would you like something "fault-Tolerant" is that? But I understand that in this case you only care about the empty catch. It is complicated because the catch may just be covering a problem elsewhere. In your case you could have a verifier to know if the email was actually sent, if not, could have another thread to try a new resend, meanwhile the registration continues.

  • 2

    We can take the opposite view. If you feel like the empty catch is satisfactory, perhaps the specific exception to be discussed should not even be being cast. Not treating the exception can be a bad sign that you don’t understand the exception or it can be a bad sign of an erroneous exception being thrown. I find it difficult an empty catch being acceptable.

  • 3

    I’d say it’s bad practice.

  • 3

    If you’re making one try{}catch(){}, why don’t you treat? or at least generate a log, for future correction.

  • @Marcoaurélioderead the exception comes from the third-party library. I did not define it in this case.

  • 1

    @Guilhermelautert soon someone will say: "It is better to leave the profession",rsrsrsrs.

  • @Wallace Maxters excuses curiosity, but what makes this exception lib and what exception does it burst? out of curiosity

  • @Wallacemaxters I know this as Pattern silenciator, it has good use to hide possible failures .... but it is great for Voce to take hours to figure out the problems.... yes very bad use

  • @Rboschini is as follows. In this part of the system, I can send an email to the student when their photos are approved. The problem is that I cannot fail to approve these photos just because the email was not sent, so I used the empty catch. Today, of course, I would have other ways to solve this, since I now have time to think of something better.But the problem is that in this system the approval of the images and the sending of the email occur together. As I was giving dick in the email once while (Brazilian servers!), I had to put this catch. The exception comes from the third-party code.

Show 12 more comments

1 answer

20


Yeah. Terrible :)

First I would need to see if that really is necessary to throw the exception. I see many cases where the exception is not the most appropriate (see more here and here).

This may be a vexatious/noisy exception (see link up there). Maybe it’s a foreign exception, which would be normal.

If everything is normal with the launch then you have to ask yourself why not do anything. Is this the action expected? Will not warn, will not log in, will not take a different course, choose an alternative, nothing like that? Will pretend that nothing happened?

If you’ve considered all this and you’re sure doing nothing is right then it’s all right. Like everything else, fixed rules are worth nothing, you have to do what’s right in that case.

It is hard to say that there is a case where swallowing the exception without doing something is right, but it can exist. When this is normal maybe the exception is the wrong mechanism. The exception name is not so by chance, it must be something exceptional, not normal. I’ve seen people defend that exception should not be used for exceptional situations, boot strength shirt in such a person.

This case seems strange that it can be ignored. "It was not so important to send the email as the request was registered", swears? So don’t send him.

It can be called anti-standard, but they are there to be used as well. This is called Exception swallowing or Hiding error.

You have the risk of doing something wrong :P If it is right for this situation there is no problem. Make sure that at no time the fact of failure not to have a treatment will not cause another problem.

There’s a problem in this particular case, it’s capturing Exception. This is very wrong, because he will swallow exceptions that need to be treated, which may even be programming errors (mess), there is serious. If you’re going to swallow it, it’s even more important to capture a very specific.

What if the exception is not so specific? Find another supplier. This one sucks :P

  • With the "exception" of my case ignoring the exception. It was a joke :D

  • I think the Beard of the Bigown can be considered the correct one, in my opinion!!!

Browser other questions tagged

You are not signed in. Login or sign up in order to post.