Haruskah saya mencatat kesalahan pada pengecualian melempar konstruktor?

Saya sedang membangun aplikasi selama beberapa bulan dan saya menyadari sebuah pola yang muncul:

logger.error(ERROR_MSG);
throw new Exception(ERROR_MSG);

Atau, saat menangkap:

try { 
    // ...block that can throw something
} catch (Exception e) {
    logger.error(ERROR_MSG, e);
    throw new MyException(ERROR_MSG, e);
}

Jadi, setiap kali saya melempar atau menangkap pengecualian, saya akan mencatatnya. Bahkan, itu hampir semua logging yang saya lakukan pada aplikasi (selain sesuatu untuk inisialisasi aplikasi).

Jadi, sebagai seorang programmer, saya menghindari pengulangan; Jadi saya memutuskan untuk memindahkan panggilan logger ke konstruksi pengecualian, jadi, setiap kali saya membangun pengecualian, semuanya akan dicatat. Saya juga bisa, tentu saja, membuat ExceptionHelper yang melempar pengecualian untuk saya, tetapi itu akan membuat kode saya lebih sulit untuk ditafsirkan dan, lebih buruk lagi, kompiler tidak akan berurusan dengan itu, gagal menyadari bahwa panggilan ke anggota itu akan lempar segera.

Jadi, apakah ini anti-pola? Jika demikian, mengapa?

Tidak yakin apakah itu memenuhi syarat sebagai anti-pola, tetapi IMO itu adalah ide yang buruk: itu tidak perlu digabungkan untuk menjalin pengecualian dengan logging.

Anda mungkin tidak selalu ingin mencatat semua contoh pengecualian yang diberikan (mungkin itu terjadi selama validasi input, yang pencatatannya mungkin banyak dan tidak menarik).

Kedua, Anda mungkin memutuskan untuk mencatat kejadian kesalahan yang berbeda dengan tingkat logging yang berbeda, pada titik mana Anda harus menentukan bahwa ketika membangun pengecualian, yang lagi-lagi merepotkan penciptaan pengecualian dengan perilaku logging.

Akhirnya, bagaimana jika pengecualian terjadi selama pencatatan pengecualian lain? Apakah Anda akan mencatatnya? Itu menjadi berantakan ...

Pilihan Anda pada dasarnya adalah:

• Tangkap, masuk dan lempar (kembali), seperti yang Anda berikan dalam contoh Anda
• Buat kelas ExceptionHelper untuk melakukan keduanya untuk Anda, tetapi Pembantu memiliki bau kode, dan saya tidak akan merekomendasikan ini juga.
• Pindahkan catch-all exception handling ke level yang lebih tinggi
• Pertimbangkan AOP untuk solusi yang lebih canggih untuk masalah lintas sektoral seperti penebangan dan penanganan pengecualian (tetapi jauh lebih kompleks daripada hanya memiliki dua jalur di blok tangkapan Anda;))

Jadi, sebagai seorang programmer, saya menghindari pengulangan [...]

Ada bahaya di sini setiap kali konsep "don't repeat yourself"ini dianggap terlalu serius hingga menjadi bau.

Untuk echo @ Dan1701

Ini adalah tentang pemisahan kekhawatiran - memindahkan logging ke pengecualian menciptakan hubungan yang erat antara pengecualian dan logging dan juga berarti bahwa Anda telah menambahkan tanggung jawab tambahan untuk pengecualian logging yang pada gilirannya dapat menciptakan dependensi untuk kelas pengecualian yang dibuatnya. tidak perlu.

Dari sudut pandang pemeliharaan Anda dapat (saya akan) berpendapat bahwa Anda menyembunyikan fakta bahwa pengecualian sedang dicatat dari pengelola (setidaknya dalam konteks seperti contoh), Anda juga mengubah konteks ( dari lokasi pengendali pengecualian ke konstruktor pengecualian) yang mungkin bukan yang Anda maksudkan.

Akhirnya Anda membuat asumsi bahwa Anda selalu ingin mencatat pengecualian, dengan cara yang persis sama, pada titik di mana dibuat / dimunculkan - yang mungkin tidak demikian. Kecuali Anda akan memiliki pengecualian logging dan non-logging yang akan menjadi sangat tidak menyenangkan dengan cepat.

Jadi dalam hal ini saya pikir "SRP" mengalahkan "KERING".

Kesalahan Anda adalah membuat pengecualian di mana Anda tidak bisa menanganinya, dan dengan demikian tidak bisa tahu apakah logging adalah bagian dari penanganan yang benar.
Ada beberapa kasus di mana Anda dapat atau harus menangani bagian dari kesalahan, yang mungkin termasuk logging, tetapi masih harus memberi sinyal kesalahan kepada penelepon. Contohnya adalah kesalahan baca yang tidak dapat diperbaiki dan sejenisnya, jika Anda melakukan pembacaan tingkat terendah, tetapi mereka umumnya memiliki kesamaan bahwa info yang dikomunikasikan kepada penelepon sangat disaring, untuk keamanan dan kegunaan.

Satu-satunya hal yang dapat Anda lakukan dalam kasus Anda, dan untuk beberapa alasan harus dilakukan, adalah menerjemahkan pengecualian yang dilemparkan ke dalam satu yang diharapkan oleh penelepon, merantai yang asli untuk konteks, dan meninggalkan hal lain dengan baik.

Singkatnya, kode Anda mendapatkan hak dan kewajiban untuk menangani sebagian pengecualian, sehingga melanggar SRP.
KERING tidak masuk ke dalamnya.

Memikirkan kembali pengecualian hanya karena Anda memutuskan untuk mencatatnya menggunakan blok tangkap (artinya pengecualian tidak berubah sama sekali) adalah ide yang buruk.

Salah satu alasan kami menggunakan pengecualian, pesan pengecualian dan penanganannya adalah agar kami tahu apa yang salah dan pengecualian yang ditulis dengan cerdik dapat mempercepat menemukan bug dengan margin yang besar.

Juga ingat, menangani pengecualian membutuhkan sumber daya yang jauh lebih banyak daripada katakan saja if, jadi Anda tidak boleh menangani semuanya terlalu sering hanya karena Anda menginginkannya. Ini berdampak pada kinerja aplikasi Anda.

Namun pendekatan yang baik untuk menggunakan pengecualian sebagai cara untuk menandai lapisan aplikasi di mana kesalahan muncul.

Pertimbangkan kode semi-pseudo berikut:

interface ICache<T, U>
{
    T GetValueByKey(U key); // may throw an CacheException
}

class FileCache<T, U> : ICache<T, U>
{
    T GetValueByKey(U key)
    {
        throw new CacheException("Could not retrieve object from FileCache::getvalueByKey. The File could not be opened. Key: " + key);
    }
}

class RedisCache<T, U> : ICache<T, U>
{
    T GetValueByKey(U key)
    {
        throw new CacheException("Could not retrieve object from RedisCache::getvalueByKey. Failed connecting to Redis server. Redis server timed out. Key: " + key);
    }
}

class CacheableInt
{
    ICache<int, int> cache;
    ILogger logger;

    public CacheableInt(ICache<int, int> cache, ILogger logger)
    {
        this.cache = cache;
        this.logger = logger;
    }

    public int GetNumber(int key) // may throw service exception
    {
        int result;

        try {
            result = this.cache.GetValueByKey(key);
        } catch (Exception e) {
            this.logger.Error(e);
            throw new ServiceException("CacheableInt::GetNumber failed, because the cache layer could not respond to request. Key: " + key);
        }

        return result;
    }
}

class CacheableIntService
{
    CacheableInt cacheableInt;
    ILogger logger;

    CacheableInt(CacheableInt cacheableInt, ILogger logger)
    {
        this.cacheableInt = cacheableInt;
        this.logger = logger;
    }

    int GetNumberAndReturnCode(int key)
    {
        int number;

        try {
            number = this.cacheableInt.GetNumber(key);
        } catch (Exception e) {
            this.logger.Error(e);
            return 500; // error code
        }

        return 200; // ok code
    }
}

Mari kita asumsikan seseorang telah memanggil GetNumberAndReturnCodedan menerima 500kode, menandakan kesalahan. Dia akan memanggil dukungan, siapa yang akan membuka file log dan melihat ini:

ERROR: 12:23:27 - Could not retrieve object from RedisCache::getvalueByKey. Failed connecting to Redis server. Redis server timed out. Key: 28
ERROR: 12:23:27 - CacheableInt::GetNumber failed, because the cache layer could not respond to request. Key: 28

Pengembang kemudian segera mengetahui lapisan perangkat lunak mana yang menyebabkan proses untuk dibatalkan dan memiliki cara mudah untuk mengidentifikasi masalah. Dalam hal ini sangat penting, karena Redis tidak boleh mundur.

Mungkin pengguna lain akan memanggil metode yang sama, juga menerima 500kode, tetapi log akan menunjukkan yang berikut:

INFO: 11:11:11- Could not retrieve object from RedisCache::getvalueByKey. Value does not exist for the key 28.
INFO: 11:11:11- CacheableInt::GetNumber failed, because the cache layer could not find any data for the key 28.

Dalam hal ini dukungan hanya dapat menanggapi pengguna bahwa permintaan itu tidak valid karena ia meminta nilai untuk ID yang tidak ada.

Ringkasan

Jika Anda menangani pengecualian, pastikan untuk menanganinya dengan cara yang benar. Pastikan juga pengecualian Anda menyertakan data / pesan yang benar di tempat pertama, mengikuti lapisan arsitektur Anda, sehingga pesan akan membantu Anda mengidentifikasi masalah yang mungkin terjadi.

Saya pikir masalahnya ada pada level yang lebih mendasar: Anda mencatat kesalahan dan membuangnya sebagai pengecualian di tempat yang sama. Itu anti-polanya. Ini berarti kesalahan yang sama dicatat berkali-kali jika tertangkap, mungkin dibungkus dengan pengecualian lain dan dilemparkan kembali.

Alih-alih ini saya sarankan untuk mencatat kesalahan bukan ketika pengecualian dibuat, tetapi ketika tertangkap. (Untuk ini, tentu saja, Anda harus memastikan selalu ada di suatu tempat.) Ketika pengecualian ditangkap, saya hanya akan mencatat stacktrace-nya jika tidak dibuang atau dibungkus sebagai penyebab pengecualian lain. Tumpukan jejak dan pesan pengecualian yang dibungkus dicatat dalam tumpukan jejak sebagai "Disebabkan oleh ...". Dan penangkap juga dapat memutuskan misalnya untuk mencoba lagi tanpa mencatat kesalahan pada kegagalan pertama, atau hanya memperlakukannya sebagai peringatan, atau apa pun.

Saya tahu ini adalah utas lama, tapi saya baru saja menemukan masalah yang sama dan menemukan solusi yang sama jadi saya akan menambahkan 2 sen saya.

Saya tidak membeli argumen pelanggaran SRP. Tidak sepenuhnya. Mari kita asumsikan 2 hal: 1. Anda benar-benar ingin mencatat pengecualian saat terjadi (pada tingkat jejak untuk dapat membuat ulang aliran program). Ini tidak ada hubungannya dengan penanganan pengecualian. 2. Anda tidak bisa atau Anda tidak akan menggunakan AOP untuk itu - saya setuju itu akan menjadi cara terbaik untuk pergi tetapi sayangnya saya terjebak dengan bahasa yang tidak menyediakan alat untuk itu.

Cara saya melihatnya, Anda pada dasarnya dijatuhi hukuman pelanggaran SRP skala besar, karena setiap kelas yang ingin melempar pengecualian harus mengetahui log tersebut. Memindahkan logging ke kelas pengecualian sebenarnya sangat mengurangi pelanggaran SRP karena sekarang hanya pengecualian yang melanggarnya dan tidak setiap kelas di basis kode.

Ini anti-pola.

Menurut pendapat saya, membuat panggilan logging di konstruktor pengecualian akan menjadi contoh berikut ini: Cacat: Konstruktor melakukan Pekerjaan Nyata .

Saya tidak akan pernah mengharapkan (atau menginginkan) konstruktor untuk melakukan panggilan layanan eksternal. Itu adalah efek samping yang sangat tidak diinginkan yang, seperti yang ditunjukkan Miško Hevery, memaksa subclass dan ejekan untuk mewarisi perilaku yang tidak diinginkan.

Dengan demikian, itu juga akan melanggar prinsip ketakjuban .

Jika Anda mengembangkan aplikasi dengan orang lain, maka efek samping ini kemungkinan tidak akan terlihat oleh mereka. Bahkan jika Anda bekerja sendirian, Anda mungkin melupakannya dan mengejutkan diri sendiri.