struct dengan nilai default tidak masuk akal

12

Dalam sistem saya saya sering beroperasi dengan kode bandara ( "YYZ", "LAX", "SFO", dll), mereka selalu dalam format yang sama persis (3 huruf, direpresentasikan sebagai huruf besar). Sistem ini biasanya menangani 25-50 kode (berbeda) ini per permintaan API, dengan lebih dari seribu total alokasi, mereka diedarkan melalui banyak lapisan aplikasi kita, dan cukup sering dibandingkan untuk kesetaraan.

Kami mulai dengan hanya memberikan string di sekitar, yang bekerja dengan baik untuk sedikit tetapi kami dengan cepat melihat banyak kesalahan pemrograman dengan mengirimkan kode yang salah di suatu tempat kode 3 digit diharapkan. Kami juga mengalami masalah di mana kami seharusnya melakukan perbandingan case-insensitive dan sebaliknya tidak, menghasilkan bug.

Dari sini, saya memutuskan untuk berhenti melewati string dan membuat Airportkelas, yang memiliki konstruktor tunggal yang mengambil dan memvalidasi kode bandara.

public sealed class Airport
{
    public Airport(string code)
    {
        if (code == null)
        {
            throw new ArgumentNullException(nameof(code));
        }

        if (code.Length != 3 || !char.IsLetter(code[0]) 
        || !char.IsLetter(code[1]) || !char.IsLetter(code[2]))
        {
            throw new ArgumentException(
                "Must be a 3 letter airport code.", 
                nameof(code));
        }

        Code = code.ToUpperInvariant();
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code;
    }

    private bool Equals(Airport other)
    {
        return string.Equals(Code, other.Code);
    }

    public override bool Equals(object obj)
    {
        return obj is Airport airport && Equals(airport);
    }

    public override int GetHashCode()
    {
        return Code?.GetHashCode() ?? 0;
    }

    public static bool operator ==(Airport left, Airport right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}

Ini membuat kode kami jauh lebih mudah untuk dipahami dan kami menyederhanakan pemeriksaan kesetaraan, penggunaan kamus / set kami. Kita sekarang tahu bahwa jika metode kita menerima Airportcontoh bahwa ia akan berperilaku seperti yang kita harapkan, itu telah menyederhanakan pemeriksaan metode kita menjadi pemeriksaan referensi nol.

Namun yang saya perhatikan adalah pengumpulan sampah berjalan lebih sering, yang saya telusuri hingga banyak contoh Airportpengumpulan.

Solusi saya untuk ini adalah untuk mengubah classmenjadi struct. Sebagian besar itu hanya perubahan kata kunci, dengan pengecualian GetHashCodedan ToString:

public override string ToString()
{
    return Code ?? string.Empty;
}

public override int GetHashCode()
{
    return Code?.GetHashCode() ?? 0;
}

Untuk menangani kasing di mana default(Airport)digunakan.

Pertanyaan saya:

  1. Apakah membuat Airportkelas atau struct merupakan solusi yang baik secara umum, atau apakah saya memecahkan masalah yang salah / menyelesaikannya dengan cara yang salah dengan membuat tipe? Jika itu bukan solusi yang baik, apa solusi yang lebih baik?

  2. Bagaimana seharusnya aplikasi saya menangani instance tempat default(Airport)digunakan? Jenis default(Airport)tidak masuk akal untuk aplikasi saya, jadi saya sudah melakukan if (airport == default(Airport) { throw ... }di tempat-tempat di mana mendapatkan instance Airport(dan Codepropertinya) sangat penting untuk operasi.

Catatan: Saya meninjau pertanyaan C # / VB struct - bagaimana cara menghindari case dengan nol nilai default, yang dianggap tidak valid untuk struktur yang diberikan? , dan Gunakan struct atau tidak sebelum mengajukan pertanyaan saya, namun saya pikir pertanyaan saya cukup berbeda untuk menjamin posnya sendiri.

Matius
sumber
7
Apakah pengumpulan sampah berdampak material pada kinerja aplikasi Anda? Dengan kata lain, apakah itu penting?
Robert Harvey
Pokoknya, ya, solusi kelas itu "bagus". Cara Anda tahu bahwa itu memecahkan masalah Anda tanpa membuat yang baru.
Robert Harvey
2
Salah satu cara Anda dapat menyelesaikan default(Airport)masalah adalah dengan tidak mengizinkan instance default. Anda dapat melakukannya dengan menulis konstruktor tanpa parameter dan melemparkan InvalidOperationExceptionatau NotImplementedExceptiondi dalamnya.
Robert Harvey
3
Sebagai tambahan, alih-alih mengonfirmasi bahwa string inisialisasi Anda sebenarnya adalah 3 karakter alfa, mengapa tidak membandingkannya dengan daftar terbatas semua kode bandara (mis., Github.com/datasets/airport-codes atau yang serupa)?
Dan Pichelman
2
Saya berani bertaruh beberapa bir bahwa ini bukan akar masalah kinerja. Laptop biasa dapat mengalokasikan dalam urutan 10 juta objek / detik.
Esben Skov Pedersen

Jawaban:

6

Pembaruan: Saya menulis ulang jawaban saya untuk mengatasi beberapa asumsi yang salah tentang C # struct, serta OP yang memberi tahu kami dalam komentar bahwa string yang diinternir sedang digunakan.


Jika Anda dapat mengontrol data yang masuk ke sistem Anda, gunakan kelas saat Anda memposting di pertanyaan Anda. Jika seseorang berlari default(Airport)mereka akan mendapatkan nullnilai kembali. Pastikan untuk menulis Equalsmetode pribadi Anda untuk mengembalikan false setiap kali membandingkan objek Bandara nol, dan kemudian biarkan NullReferenceExceptionterbang di tempat lain dalam kode.

Namun, jika Anda mengambil data ke dalam sistem dari sumber yang tidak Anda kontrol, Anda tidak perlu ingin membuat seluruh utas macet. Dalam hal ini struct ideal untuk fakta sederhana default(Airport)akan memberi Anda sesuatu selain nullpointer. Buat nilai yang jelas untuk mewakili "tidak ada nilai" atau "nilai default" sehingga Anda memiliki sesuatu untuk dicetak di layar atau dalam file log (seperti "---" misalnya). Bahkan, saya hanya akan menjaga codeprivasi dan tidak mengekspos Codeproperti sama sekali - hanya fokus pada perilaku di sini.

public struct Airport
{
    private string code;

    public Airport(string code)
    {
        // Check `code` for validity, throw exceptions if not valid

        this.code = code;
    }

    public override string ToString()
    {
        return code ?? (code = "---");
    }

    // int GetHashcode()

    // bool Equals(...)

    // bool operator ==(...)

    // bool operator !=(...)

    private bool Equals(Airport other)
    {
        if (other == null)
            // Even if this method is private, guard against null pointers
            return false;

        if (ToString() == "---" || other.ToString() == "---")
            // "Default" values should never match anything, even themselves
            return false;

        // Do a case insensitive comparison to enforce logic that airport
        // codes are not case sensitive
        return string.Equals(
            ToString(),
            other.ToString(),
            StringComparison.InvariantCultureIgnoreCase);
    }
}

Skenario kasus yang lebih buruk mengkonversi default(Airport)ke string mencetak "---"dan mengembalikan false jika dibandingkan dengan kode bandara lainnya yang valid. Kode bandara "default" tidak cocok dengan apa pun, termasuk kode bandara standar lainnya.

Ya, struct dimaksudkan sebagai nilai yang dialokasikan pada stack, dan setiap pointer untuk menumpuk memori pada dasarnya meniadakan keuntungan kinerja struct, tetapi dalam hal ini nilai default dari sebuah struct memiliki makna dan memberikan beberapa resistensi peluru tambahan ke seluruh aplikasi.

Saya akan sedikit membengkokkan aturan di sini, karena itu.


Jawaban Asli (dengan beberapa kesalahan faktual)

Jika Anda dapat mengontrol data yang masuk ke sistem Anda, saya akan melakukan seperti yang disarankan Robert Harvey dalam komentar: Buat konstruktor tanpa parameter dan berikan pengecualian saat dipanggil. Ini mencegah data yang tidak valid memasuki sistem melalui default(Airport).

public Airport()
{
    throw new InvalidOperationException("...");
}

Namun, jika Anda mengambil data ke dalam sistem dari sumber yang tidak Anda kontrol, Anda tidak perlu ingin membuat seluruh utas macet. Dalam hal ini Anda dapat membuat kode bandara yang tidak valid, tetapi membuatnya tampak seperti kesalahan nyata. Ini akan melibatkan pembuatan konstruktor tanpa parameter dan pengaturan Codeke sesuatu seperti "---":

public Airport()
{
    Code = "---";
}

Karena Anda menggunakan stringsebagai Kode, tidak ada gunanya menggunakan struct. Struct akan dialokasikan pada stack, hanya untuk Codedialokasikan sebagai pointer ke string di memori tumpukan, jadi tidak ada perbedaan di sini antara kelas dan struct.

Jika Anda mengubah kode bandara menjadi array 3 item dari char maka struct akan sepenuhnya dialokasikan pada stack. Meski begitu volume data tidak terlalu besar untuk membuat perbedaan.

Greg Burghardt
sumber
Jika aplikasi saya menggunakan string yang diinternir untuk Codeproperti, apakah itu akan mengubah pembenaran Anda tentang titik string yang ada di memori tumpukan?
Matius
@ Matthew: Apakah menggunakan kelas memberi Anda masalah kinerja? Jika tidak, balikkan koin untuk memutuskan mana yang akan digunakan.
Greg Burghardt
4
@ Matthew: Sungguh yang terpenting adalah Anda memusatkan logika yang merepotkan untuk menormalkan kode dan perbandingan. Setelah itu "kelas versus struct" hanyalah diskusi akademis, sampai Anda mengukur dampak yang cukup besar dalam kinerja untuk membenarkan waktu pengembang tambahan untuk berdiskusi akademik.
Greg Burghardt
1
Itu benar, saya tidak keberatan berdiskusi akademik dari waktu ke waktu jika itu membantu saya menciptakan solusi yang lebih baik di masa depan.
Matius
@ Matthew: Yup, Anda memang benar. Mereka mengatakan "bicara itu murah." Ini tentu lebih murah daripada tidak berbicara dan membangun sesuatu yang buruk. :)
Greg Burghardt
13

Gunakan pola Flyweight

Karena Airport, dengan benar, tidak dapat diubah, tidak perlu membuat lebih dari satu instance dari yang tertentu, katakanlah, SFO. Gunakan Hashtable atau yang serupa (perhatikan, saya seorang pria Java, bukan C # jadi detail yang tepat mungkin berbeda), untuk men-cache Bandara ketika mereka dibuat. Sebelum membuat yang baru, periksa di Hashtable. Anda tidak pernah membebaskan Bandara, jadi GC tidak perlu membebaskan mereka.

Satu keuntungan kecil tambahan (setidaknya di Jawa, tidak yakin tentang C #) adalah bahwa Anda tidak perlu menulis equals()metode, yang sederhana ==akan dilakukan. Sama untuk hashcode().

pengguna949300
sumber
3
Penggunaan yang sangat baik dari pola bobot terbang.
Neil
2
Dengan asumsi OP terus menggunakan struct dan bukan kelas, bukankah pemagangan string sudah menangani nilai-nilai string yang dapat digunakan kembali? Struct sudah hidup di stack, string sudah digunakan kembali untuk menghindari nilai duplikat dalam memori. Apa manfaat tambahan yang akan diperoleh dari pola kelas terbang?
Flater
Sesuatu yang harus diwaspadai. Jika bandara ditambahkan atau dihapus, Anda ingin membangun cara menyegarkan daftar statis ini tanpa me-restart aplikasi atau menggunakannya kembali. Bandara tidak sering ditambahkan atau dihapus, tetapi pemilik bisnis cenderung sedikit kesal ketika perubahan sederhana menjadi rumit. "Tidak bisakah aku menambahkannya di suatu tempat ?! Mengapa kita harus menjadwalkan rilis / aplikasi restart dan membuat pelanggan kita tidak nyaman?" Tapi saya juga berpikir untuk menggunakan semacam cache statis pada awalnya juga.
Greg Burghardt
@Flater Poin yang masuk akal. Saya akan mengatakan kurang perlu untuk programmer junior untuk alasan tentang tumpukan vs tumpukan. Plus lihat tambahan saya - tidak perlu menulis equals ().
user949300
1
@Greg Burghardt Jika getAirportOrCreate()kode disinkronkan dengan benar, tidak ada alasan teknis Anda tidak dapat membuat Bandara baru yang diperlukan selama runtime. Mungkin ada alasan bisnis.
user949300
3

Saya bukan programmer yang sangat canggih, tetapi bukankah ini akan menjadi penggunaan yang sempurna untuk Enum?

Ada berbagai cara untuk membangun kelas enum dari daftar atau string. Ini salah satu yang pernah saya lihat di masa lalu, tidak yakin apakah itu cara terbaik.

https://blog.kloud.com.au/2016/06/17/converting-webconfig-values-into-enum-or-list/

Adam B
sumber
2
Ketika ada ribuan nilai yang berpotensi (seperti halnya dengan kode bandara), enum tidak praktis.
Ben Cottrell
Ya, tetapi tautan yang saya poskan adalah cara memuat string sebagai enum. Berikut tautan lain untuk memuat tabel pencarian sebagai enum. Mungkin sedikit kerja, tetapi akan mengambil keuntungan dari kekuatan enum. exceptionnotfound.net/…
Adam B
1
Atau daftar kode yang valid dapat diambil dari database atau file. Kemudian kode bandara baru saja diperiksa untuk berada di antara daftar itu. Ini adalah apa yang biasanya Anda lakukan ketika Anda tidak ingin lagi meng-hardcode nilai-nilai dan / atau daftar menjadi terlalu lama untuk dikelola.
Neil
@BenCottrell untuk itulah templat kode gen dan T4.
RubberDuck
3

Salah satu alasan Anda melihat lebih banyak aktivitas GC adalah karena Anda membuat string kedua sekarang - .ToUpperInvariant()versi dari string asli. String asli memenuhi syarat untuk GC tepat setelah konstruktor berjalan dan yang kedua memenuhi syarat pada saat yang sama dengan Airportobjek. Anda mungkin dapat menguranginya dengan cara yang berbeda (perhatikan parameter ketiga untuk string.Equals()):

public sealed class Airport : IEquatable<Airport>
{
    public Airport(string code)
    {
        if (code == null)
        {
            throw new ArgumentNullException(nameof(code));
        }

        if (code.Length != 3 || !char.IsLetter(code[0])
                             || !char.IsLetter(code[1]) || !char.IsLetter(code[2]))
        {
            throw new ArgumentException(
                "Must be a 3 letter airport code.",
                nameof(code));
        }

        Code = code;
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code; // TODO: Upper-case it here if you really need to for display.
    }

    public bool Equals(Airport other)
    {
        return string.Equals(Code, other?.Code, StringComparison.InvariantCultureIgnoreCase);
    }

    public override bool Equals(object obj)
    {
        return obj is Airport airport && Equals(airport);
    }

    public override int GetHashCode()
    {
        return Code.GetHashCode();
    }

    public static bool operator ==(Airport left, Airport right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}
Jesse C. Slicer
sumber
Tidakkah ini menghasilkan kode hash yang berbeda untuk Bandara yang sama (tetapi bermodal berbeda)?
Hero Wanders
Ya, saya bayangkan begitu. Dangit.
Jesse C. Slicer
Ini adalah poin yang sangat bagus, tidak pernah dipikirkan, saya akan melihat membuat perubahan ini.
Matius
1
Sehubungan dengan itu GetHashCode, sebaiknya hanya menggunakan StringComparer.OrdinalIgnoreCase.GetHashCode(Code)atau serupa
Matius