Mengapa tangkapan kosong menghalangi ide yang buruk? [Tutup]

194

Saya baru saja melihat pertanyaan tentang coba-tangkap , yang dikatakan orang (termasuk Jon Skeet) blok tangkap kosong adalah ide yang sangat buruk? Kenapa ini? Apakah tidak ada situasi di mana tangkapan kosong bukan keputusan desain yang salah?

Maksud saya, misalnya, kadang-kadang Anda ingin mendapatkan beberapa info tambahan dari suatu tempat (layanan web, basis data) dan Anda benar-benar tidak peduli apakah Anda akan mendapatkan info ini atau tidak. Jadi Anda mencoba untuk mendapatkannya, dan jika sesuatu terjadi, tidak apa-apa, saya hanya akan menambahkan "tangkapan (Pengecualian diabaikan) {}" dan itu saja

Samuel Carrijo
sumber
53
Saya akan menulis komentar yang menjelaskan mengapa harus kosong.
Mehrdad Afshari
5
... atau setidaknya mencatat bahwa itu tertangkap.
matt b
2
@ocdecio: hindari untuk kode pembersihan , jangan menghindarinya secara umum.
John Saunders
1
@ocdecio: Kasus lain untuk menghindari penggunaan try..catch adalah ketika Anda menangkap pengecualian untuk jenis kegagalan yang diketahui. mis. pengecualian konversi numerik
MPritchard
1
@ocdecio - coba..hasil lebih baik dari mencoba..kurang tangkapan karena kesalahan terus menumpuk - untuk ditangani oleh kerangka kerja, atau untuk mematikan program dan ditampilkan kepada pengguna - kedua hasil lebih baik daripada " kegagalan diam ".
Nate

Jawaban:

298

Biasanya try-catch kosong adalah ide yang buruk karena Anda diam-diam menelan kondisi kesalahan dan kemudian melanjutkan eksekusi. Kadang-kadang ini mungkin hal yang benar untuk dilakukan, tetapi seringkali itu merupakan pertanda bahwa pengembang melihat pengecualian, tidak tahu harus berbuat apa, dan menggunakan tangkapan kosong untuk membungkam masalah.

Ini sama dengan pemrograman menempatkan pita hitam di atas lampu peringatan mesin.

Saya percaya bahwa cara Anda menangani pengecualian tergantung pada lapisan perangkat lunak yang Anda gunakan: Pengecualian di Hutan Hujan .

Ned Batchelder
sumber
4
Dalam keadaan yang benar-benar langka di mana saya benar - benar tidak memerlukan pengecualian dan penebangan tidak tersedia untuk beberapa alasan, saya pastikan untuk berkomentar bahwa itu disengaja - dan mengapa itu tidak diperlukan, dan menegaskan kembali bahwa saya masih lebih suka untuk login jika itu layak di lingkungan itu. Pada dasarnya saya membuat komentar LEBIH berfungsi daripada penebangan seandainya itu menjadi pilihan dalam keadaan itu. Untungnya saya punya 2 klien untuk siapa ini masalah, dan itu hanya ketika mengirim email yang tidak penting dari situs web mereka.
John Rudy
37
Juga suka analogi - dan seperti semua aturan, ada pengecualian. Misalnya, tidak apa-apa untuk menggunakan pita hitam di atas jam yang berkedip pada VCR Anda jika Anda tidak pernah berniat untuk melakukan rekaman waktunya (untuk semua Anda orang tua yang ingat apa itu VCR).
Michael Burr
3
Seperti halnya penyimpangan dari praktik yang diterima, lakukan jika perlu dan dokumentasikan agar orang berikutnya tahu apa yang Anda lakukan dan mengapa.
David Thornley
5
@Jason: setidaknya, Anda harus menyertakan komentar terperinci yang menjelaskan mengapa Anda membungkam pengecualian.
Ned Batchelder
1
Jawaban ini mengasumsikan dua hal: 1. Bahwa pengecualian yang dilemparkan adalah benar-benar bermakna dan bahwa orang yang menulis kode yang melempar tahu apa yang dia lakukan; 2. Bahwa pengecualian memang merupakan "kondisi kesalahan" dan tidak disalahgunakan sebagai aliran kontrol (meskipun ini adalah kasus yang lebih spesifik dari poin pertama saya).
Boris B.
38

Mereka adalah ide yang buruk secara umum karena ini adalah kondisi yang benar-benar langka di mana kegagalan (kondisi luar biasa, lebih umum) bertemu dengan benar tanpa respons sama sekali. Selain itu, catchblok kosong adalah alat yang umum digunakan oleh orang yang menggunakan mesin pengecualian untuk memeriksa kesalahan yang seharusnya mereka lakukan terlebih dahulu.

Mengatakan bahwa itu selalu buruk itu tidak benar ... itu benar sangat sedikit. Mungkin ada keadaan di mana Anda tidak peduli bahwa ada kesalahan atau bahwa keberadaan kesalahan entah bagaimana menunjukkan bahwa Anda tetap tidak bisa berbuat apa-apa (misalnya, ketika menulis kesalahan sebelumnya ke file log teks dan Anda mendapatkan IOException, artinya Anda tidak dapat menulis kesalahan baru).

Adam Robinson
sumber
11

Saya tidak akan merentangkan hal-hal sejauh mengatakan bahwa siapa yang menggunakan blok tangkap kosong adalah programmer yang buruk dan tidak tahu apa yang dia lakukan ...

Saya menggunakan blok tangkap kosong jika perlu. Terkadang programmer perpustakaan yang saya konsumsi tidak tahu apa yang dia lakukan dan melempar pengecualian bahkan dalam situasi ketika tidak ada yang membutuhkannya.

Sebagai contoh, pertimbangkan beberapa perpustakaan http server, saya tidak peduli jika server melempar pengecualian karena klien telah terputus dan index.htmltidak dapat dikirim.

lubos hasko
sumber
6
Anda tentu terlalu menggeneralisasi. Hanya karena Anda tidak membutuhkannya, bukan berarti tidak ada yang melakukannya. Bahkan jika sama sekali tidak ada yang bisa dilakukan sebagai tanggapan, ada seseorang di suatu tempat dengan persyaratan untuk mengumpulkan statistik tentang koneksi yang ditinggalkan. Jadi cukup kasar untuk menganggap "programmer perpustakaan tidak tahu apa yang dia lakukan".
Ben Voigt
8

Ada beberapa contoh langka di mana hal itu dapat dibenarkan. Dengan Python Anda sering melihat konstruksi seperti ini:

try:
    result = foo()
except ValueError:
    result = None

Jadi mungkin OK (tergantung pada aplikasi Anda) untuk melakukan:

result = bar()
if result == None:
    try:
        result = foo()
    except ValueError:
        pass # Python pass is equivalent to { } in curly-brace languages
 # Now result == None if bar() returned None *and* foo() failed

Dalam proyek .NET baru-baru ini, saya harus menulis kode untuk menghitung DLL plugin untuk menemukan kelas yang mengimplementasikan antarmuka tertentu. Bit kode yang relevan (dalam VB.NET, maaf) adalah:

    For Each dllFile As String In dllFiles
        Try
            ' Try to load the DLL as a .NET Assembly
            Dim dll As Assembly = Assembly.LoadFile(dllFile)
            ' Loop through the classes in the DLL
            For Each cls As Type In dll.GetExportedTypes()
                ' Does this class implement the interface?
                If interfaceType.IsAssignableFrom(cls) Then

                    ' ... more code here ...

                End If
            Next
        Catch ex As Exception
            ' Unable to load the Assembly or enumerate types -- just ignore
        End Try
    Next

Meskipun dalam kasus ini, saya akui bahwa mencatat kegagalan di suatu tempat mungkin akan menjadi peningkatan.

Daniel Pryden
sumber
Saya menyebut log4net sebagai salah satu contoh sebelum saya melihat jawaban Anda.
Scott Lawrence
7

Blok tangkapan kosong biasanya dimasukkan karena pembuat kode tidak benar-benar tahu apa yang mereka lakukan. Di organisasi saya, blok tangkap kosong harus menyertakan komentar mengapa tidak melakukan apa pun dengan pengecualian adalah ide yang baik.

Pada catatan terkait, kebanyakan orang tidak tahu bahwa blok percobaan {} dapat diikuti dengan tangkapan {} atau akhirnya {}, hanya diperlukan satu.

Justin
sumber
1
+1 Saya akan menekankan 'atau' dalam kalimat terakhir untuk dampak tambahan
MPritchard
Namun, paragraf kedua itu tergantung pada bahasa, bukan?
David Z
3
kecuali tentu saja Anda sedang berbicara tentang IE6 atau IE7 ;-) ... mana menangkap IS diperlukan atau akhirnya tidak mengeksekusi. (ini diperbaiki di IE8 btw)
scunliffe
Sangat benar. Mungkin patut menyoroti bahasa. Saya percaya. Net baik
MPritchard
7

Pengecualian hanya boleh dilemparkan jika benar-benar ada pengecualian - sesuatu terjadi di luar norma. Pada dasarnya blok tangkap kosong mengatakan "sesuatu yang buruk sedang terjadi, tapi aku tidak peduli". Ini ide yang buruk.

Jika Anda tidak ingin menangani pengecualian, biarkan merambat ke atas hingga mencapai beberapa kode yang bisa mengatasinya. Jika tidak ada yang dapat menangani pengecualian, itu harus menurunkan aplikasi.

Reed Copsey
sumber
3
Terkadang Anda tahu bahwa itu tidak akan mempengaruhi apa pun. Lalu maju dan lakukan, dan komentari sehingga orang berikutnya tidak hanya berpikir Anda mengacaukan karena Anda tidak kompeten.
David Thornley
Ya, ada waktu dan tempat untuk semuanya. Secara umum, saya pikir blok tangkapan kosong yang baik adalah pengecualian dari aturan - ini adalah kasus langka di mana Anda ingin benar-benar mengabaikan pengecualian (biasanya, IMO, ini adalah hasil dari penggunaan pengecualian yang buruk).
Reed Copsey
2
@ David Thornley: Bagaimana Anda tahu itu tidak akan mempengaruhi apa pun jika Anda setidaknya tidak memeriksa pengecualian untuk menentukan apakah itu kesalahan yang diharapkan dan tidak berarti atau yang seharusnya disebarkan ke atas?
Dave Sherohman
2
@ Dave: Sementara catch (Exception) {}itu ide yang buruk, catch (SpecificExceptionType) {}mungkin baik-baik saja. Programmer DID memeriksa pengecualian, menggunakan tipe informasi dalam klausa tangkapan.
Ben Voigt
7

I think it's okay if you catch a particular exception type of which you know it's only going to be raised for one particular reason, and you expect that exception and really don't need to do anything about it.

But even in that case, a debug message might be in order.

balpha
sumber
6

Per Josh Bloch - Item 65: Don't ignore Exceptions of Effective Java:

  1. An empty catch block defeats the purpose of exceptions
  2. At the very least, the catch block should contain a comment explaining why it is appropriate to ignore the exception.
KrishPrabakar
sumber
3

An empty catch block is essentially saying "I don't want to know what errors are thrown, I'm just going to ignore them."

It's similar to VB6's On Error Resume Next, except that anything in the try block after the exception is thrown will be skipped.

Which doesn't help when something then breaks.

Powerlord
sumber
I'd actually say "On Error Resume Next" is worse - it'll just try the next line regardless. An empty catch will jump to pass the closing brace of the try statement
MPritchard
Good point. I should probably note that in my response.
Powerlord
1
This is not exactly true, if you have an empty try...catch with a specific exception type, then it will ignore just this type of exception, and that might be legitimate...
Meta-Knight
But if you know a particular type of exception is being thrown, it seems like you might have enough information to write your logic in a way that avoids the use of try-catch to deal with the situation.
Scott Lawrence
@Scott: Certain languages (i.e. Java) have checked exceptions... exceptions you're forced to write catch blocks for.
Powerlord
3

This goes hand-in-hand with, "Don't use exceptions to control program flow.", and, "Only use exceptions for exceptional circumstances." If these are done, then exceptions should only be occurring when there's a problem. And if there's a problem, you don't want to fail silently. In the rare anomalies where it's not necessary to handle the problem you should at least log the exception, just in case the anomaly becomes no longer an anomaly. The only thing worse than failing is failing silently.

Imagist
sumber
2

I think a completely empty catch block is a bad idea because there is no way to infer that ignoring the exception was the intended behavior of the code. It is not necessarily bad to swallow an exception and return false or null or some other value in some cases. The .net framework has many "try" methods that behave this way. As a rule of thumb if you swallow an exception, add a comment and a log statement if the application supports logging.

complexcipher
sumber
1

Because if an exception is thrown you won't ever see it - failing silently is the worst possible option - you'll get erroneous behavior and no idea to look where it's happening. At least put a log message there! Even if it's something that 'can never happen'!

Nate
sumber
1

Empty catch blocks are an indication of a programmer not knowing what to do with an exception. They are suppressing the exception from possibly bubbling up and being handled correctly by another try block. Always try and do something with the exception you are catching.

Dan
sumber
1

I find the most annoying with empty catch statements is when some other programmer did it. What I mean is when you need to debug code from somebody else any empty catch statements makes such an undertaking more difficult then it need to be. IMHO catch statements should always show some kind of error message - even if the error is not handled it should at least detect it (alt. on only in debug mode)

AndersK
sumber
1

It's probably never the right thing because you're silently passing every possible exception. If there's a specific exception you're expecting, then you should test for it, rethrow if it's not your exception.

try
{
    // Do some processing.
}
catch (FileNotFound fnf)
{
    HandleFileNotFound(fnf);
}
catch (Exception e)
{
    if (!IsGenericButExpected(e))
        throw;
}

public bool IsGenericButExpected(Exception exception)
{
    var expected = false;
    if (exception.Message == "some expected message")
    {
        // Handle gracefully ... ie. log or something.
        expected = true;
    }

    return expected;
}
xanadont
sumber
1
That's not really a standard pattern you're advising people to use there. Most people would just catch the Exceptions of whatever types they are expecting, and not the ones they're not. I can see some circumstances where your approach would be valid, just be careful of posting in a forum like this where people tend to be readng things like this because they don't understand in the first place ;)
MPritchard
My intent was not that IsKnownException() is checking the type of the exception (yes, you should do that by different catch blocks), rather it's checking if it's an expected exception. I'll edit to make it more clear.
xanadont
I was originally thinking about COMExceptions. You can test for a specific ErrorCode and handle the codes you expect. I do this often when programming with ArcOjbects.
xanadont
2
-1 Making decisions in the code based on the exception message is a really bad idea. The exact message is rarely documented and could change at any time; it's an implementation detail. Or the message could be based on a localizable resource string. In any case it's only meant to be read by a human.
Wim Coenen
Eek. The exception.Message could be localized and thus not what you expect. This is just wrong.
frankhommers
1

Generally, you should only catch the exceptions you can actually handle. That means be as specific as possible when catching exceptions. Catching all exceptions is rarely a good idea and ignoring all exceptions is almost always a very bad idea.

I can only think of a few instances where an empty catch block has some meaningful purpose. If whatever specific exception, you are catching is "handled" by just reattempting the action there would be no need to do anything in the catch block. However, it would still be a good idea to log the fact that the exception occurred.

Another example: CLR 2.0 changed the way unhandled exceptions on the finalizer thread are treated. Prior to 2.0 the process was allowed to survive this scenario. In the current CLR the process is terminated in case of an unhandled exception on the finalizer thread.

Keep in mind that you should only implement a finalizer if you really need one and even then you should do a little as possible in the finalizer. But if whatever work your finalizer must do could throw an exception, you need to pick between the lesser of two evils. Do you want to shut down the application due to the unhandled exception? Or do you want to proceed in a more or less undefined state? At least in theory the latter may be the lesser of two evils in some cases. In those case the empty catch block would prevent the process from being terminated.

Brian Rasmussen
sumber
1
I mean, for instance, sometimes you want to get some additional info from somewhere (webservice, database) and you really don't care if you'll get this info or not. So you try to get it, and if anything happens, that's ok, I'll just add a "catch (Exception ignored) {}" and that's all

So, going with your example, it's a bad idea in that case because you're catching and ignoring all exceptions. If you were catching only EInfoFromIrrelevantSourceNotAvailable and ignoring it, that would be fine, but you're not. You're also ignoring ENetworkIsDown, which may or may not be important. You're ignoring ENetworkCardHasMelted and EFPUHasDecidedThatOnePlusOneIsSeventeen, which are almost certainly important.

An empty catch block is not an issue if it's set up to only catch (and ignore) exceptions of certain types which you know to be unimportant. The situations in which it's a good idea to suppress and silently ignore all exceptions, without stopping to examine them first to see whether they're expected/normal/irrelevant or not, are exceedingly rare.

Dave Sherohman
sumber
1

There are situations where you might use them, but they should be very infrequent. Situations where I might use one include:

  • exception logging; depending on context you might want an unhandled exception or message posted instead.

  • looping technical situations, like rendering or sound processing or a listbox callback, where the behaviour itself will demonstrate the problem, throwing an exception will just get in the way, and logging the exception will probably just result in 1000's of "failed to XXX" messages.

  • programs that cannot fail, although they should still at least be logging something.

for most winforms applications, I have found that it suffices to have a single try statement for every user input. I use the following methods: (AlertBox is just a quick MessageBox.Show wrapper)

  public static bool TryAction(Action pAction)
  {
     try { pAction(); return true; }
     catch (Exception exception)
     {
        LogException(exception);
        return false;
     }
  }

  public static bool TryActionQuietly(Action pAction)
  {
     try { pAction(); return true; }
     catch(Exception exception)
     {
        LogExceptionQuietly(exception);
        return false;
     }
  }

  public static void LogException(Exception pException)
  {
     try
     {
        AlertBox(pException, true);
        LogExceptionQuietly(pException);
     }
     catch { }
  }

  public static void LogExceptionQuietly(Exception pException)
  {
     try { Debug.WriteLine("Exception: {0}", pException.Message); } catch { }
  }

Then every event handler can do something like:

  private void mCloseToolStripMenuItem_Click(object pSender, EventArgs pEventArgs)
  {
     EditorDefines.TryAction(Dispose);
  }

or

  private void MainForm_Paint(object pSender, PaintEventArgs pEventArgs)
  {
     EditorDefines.TryActionQuietly(() => Render(pEventArgs));
  }

Theoretically, you could have TryActionSilently, which might be better for rendering calls so that an exception doesn't generate an endless amount of messages.

Dave Cousineau
sumber
1

If you dont know what to do in catch block, you can just log this exception, but dont leave it blank.

        try
        {
            string a = "125";
            int b = int.Parse(a);
        }
        catch (Exception ex)
        {
            Log.LogError(ex);
        }
Andzej Maciusovic
sumber
Why was this downvoted?
Vino
0

You should never have an empty catch block. It is like hiding a mistake you know about. At the very least you should write out an exception to a log file to review later if you are pressed for time.

Chadit
sumber