Struktur mit unsinnigem Standardwert

12

In meinem System häufig mit Flughafen - Codes arbeiten I ( "YYZ", "LAX", "SFO", etc.), sind sie immer in dem exakt gleichen Format (3 Buchstaben, als Groß dargestellt). Das System verarbeitet in der Regel 25 bis 50 dieser (unterschiedlichen) Codes pro API-Anforderung, wobei insgesamt über tausend Zuweisungen vorgenommen werden. Sie werden durch viele Ebenen unserer Anwendung geleitet und häufig auf Gleichheit verglichen.

Wir haben angefangen, indem wir nur Strings herumgereicht haben, was ein bisschen gut funktioniert hat, aber wir haben schnell viele Programmierfehler bemerkt, indem wir irgendwo einen falschen Code eingegeben haben, wo der dreistellige Code erwartet wurde. Wir sind auch auf Probleme gestoßen, bei denen wir einen Vergleich durchführen sollten, bei dem die Groß- und Kleinschreibung nicht berücksichtigt wurde. Dies führte zu Fehlern.

Aus diesem Grund habe ich beschlossen, keine Zeichenfolgen mehr weiterzugeben und eine AirportKlasse mit einem einzigen Konstruktor zu erstellen , der den Flughafencode aufnimmt und validiert.

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);
    }
}

Dies machte unseren Code viel verständlicher und wir vereinfachten unsere Gleichheitsprüfungen, Wörterbuch- / Satzverwendungen. Wir wissen jetzt, dass, wenn unsere Methoden eine AirportInstanz akzeptieren , die sich wie erwartet verhält, dies unsere Methodenprüfungen zu einer Nullreferenzprüfung vereinfacht hat.

Was mir jedoch aufgefallen ist, ist, dass die Speicherbereinigung viel häufiger ausgeführt wird, was ich auf viele Fälle zurückgeführt habe, in denen Airportsie gesammelt wurde.

Meine Lösung hierfür war, die classin eine umzuwandeln struct. Meistens handelte es sich nur um eine Keyword-Änderung, mit Ausnahme von GetHashCodeund ToString:

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

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

Um den Fall zu behandeln, in dem default(Airport)verwendet wird.

Meine Fragen:

  1. War das Erstellen einer AirportKlasse oder Struktur im Allgemeinen eine gute Lösung, oder löse ich das falsche Problem / löse ich es auf die falsche Weise, indem ich den Typ erstelle? Wenn es keine gute Lösung ist, was ist eine bessere Lösung?

  2. Wie soll meine Anwendung mit Fällen umgehen, in denen die default(Airport)verwendet wird? Eine Art von default(Airport)ist für meine Anwendung unsinnig, daher habe ich if (airport == default(Airport) { throw ... }an Stellen gearbeitet, an denen das Abrufen einer Instanz von Airport(und ihrer CodeEigenschaft) für den Vorgang von entscheidender Bedeutung ist.

Anmerkungen: Ich habe die Fragen zur C # / VB-Struktur überprüft . Und Verwenden Struktur oder nicht , bevor meine Frage zu stellen, aber ich denke , meine Fragen unterschiedlich genug sind , seinen eigenen Beitrag zu rechtfertigen.

Matthew
quelle
7
Hat die Garbage Collection einen wesentlichen Einfluss auf die Leistung Ihrer Anwendung? Mit anderen Worten, spielt es eine Rolle?
Robert Harvey
Auf jeden Fall war die Klassenlösung eine "gute". So, wie Sie es kennen, ist es Ihr Problem gelöst, ohne neue zu erstellen.
Robert Harvey
2
Eine Möglichkeit, das default(Airport)Problem zu lösen , besteht darin, Standardinstanzen einfach zu deaktivieren. Sie können dies tun, indem Sie einen parameterlosen Konstruktor schreiben und werfen InvalidOperationExceptionoder hinein NotImplementedException.
Robert Harvey
3
Anstatt zu bestätigen, dass Ihre Initialisierungszeichenfolge tatsächlich aus 3 alphanumerischen Zeichen besteht, vergleichen Sie sie einfach mit der endlichen Liste aller Flughafencodes (z. B. github.com/datasets/airport-codes oder ähnlich).
Dan Pichelman
2
Ich bin bereit, mehrere Biere zu wetten, dass dies nicht die Wurzel eines Leistungsproblems ist. Ein normaler Laptop kann in der Reihenfolge 10M Objekte / Sekunde zuordnen.
Esben Skov Pedersen

Antworten:

6

Update: Ich habe meine Antwort umgeschrieben, um einige falsche Annahmen zu C # -Strukturen sowie das OP zu berücksichtigen, das uns in Kommentaren darüber informiert, dass interne Zeichenfolgen verwendet werden.


Wenn Sie die in Ihr System eingehenden Daten steuern können, verwenden Sie eine Klasse, wie Sie sie in Ihrer Frage angegeben haben. Wenn jemand rennt default(Airport), bekommt er einen nullWert zurück. Stellen Sie sicher, dass Sie Ihre private EqualsMethode so schreiben , dass sie false zurückgibt, wenn Sie null Airport-Objekte vergleichen, und lassen Sie die NullReferenceExceptions dann an einer anderen Stelle im Code fliegen.

Wenn Sie jedoch Daten aus Quellen in das System übernehmen, die Sie nicht kontrollieren, müssen Sie nicht den gesamten Thread zum Absturz bringen. In diesem Fall ist eine Struktur ideal, denn die einfache Tatsache default(Airport)gibt Ihnen etwas anderes als einen nullZeiger. Stellen Sie einen offensichtlichen Wert für "kein Wert" oder "Standardwert" ein, damit Sie etwas auf dem Bildschirm oder in einer Protokolldatei drucken können (z. B. "---"). Tatsächlich würde ich nur das codePrivate behalten und eine CodeImmobilie überhaupt nicht aussetzen - mich nur auf das Verhalten konzentrieren.

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);
    }
}

Im schlimmsten Fall wird die Konvertierung default(Airport)in eine Zeichenfolge gedruckt "---"und im Vergleich zu anderen gültigen Flughafencodes als falsch zurückgegeben. Jeder "Standard" -Flughafencode stimmt mit nichts überein, einschließlich anderer Standard-Flughafencodes.

Ja, Strukturen sind Werte, die auf dem Stack zugewiesen sind, und alle Zeiger auf den Heap-Speicher negieren im Grunde die Leistungsvorteile von Strukturen. In diesem Fall hat der Standardwert einer Struktur jedoch eine Bedeutung und bietet dem Rest des einen zusätzlichen Aufzählungsschutz Anwendung.

Ich würde die Regeln deswegen hier ein wenig verbiegen.


Originalantwort (mit einigen sachlichen Fehlern)

Wenn Sie die in Ihr System eingehenden Daten steuern können, würde ich wie in den Kommentaren von Robert Harvey vorgeschlagen vorgehen: Erstellen Sie einen parameterlosen Konstruktor und lösen Sie eine Ausnahme aus, wenn er aufgerufen wird. Dies verhindert, dass ungültige Daten über in das System gelangen default(Airport).

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

Wenn Sie jedoch Daten aus Quellen in das System übernehmen, die Sie nicht kontrollieren, müssen Sie nicht den gesamten Thread zum Absturz bringen. In diesem Fall können Sie einen ungültigen Flughafencode erstellen, der jedoch als offensichtlicher Fehler erscheint. Dazu müsste ein parameterloser Konstruktor erstellt und der CodeWert auf "---" gesetzt werden:

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

Da Sie a stringals Code verwenden, hat die Verwendung einer Struktur keinen Sinn. Die Struktur wird auf dem Stapel zugewiesen, nur um den Codeals Zeiger auf eine Zeichenfolge im Heapspeicher zugewiesenen Wert zu haben. Daher besteht hier kein Unterschied zwischen Klasse und Struktur.

Wenn Sie den Flughafencode in ein Array mit 3 Zeichen ändern, wird dem Stapel eine Struktur vollständig zugewiesen. Selbst dann ist das Datenvolumen nicht so groß, um einen Unterschied zu machen.

Greg Burghardt
quelle
Wenn meine Anwendung internierte Zeichenfolgen für die CodeEigenschaft verwenden würde, würde dies Ihre Rechtfertigung in Bezug auf die Position der Zeichenfolge im Heapspeicher ändern?
Matthew
@Matthew: Gibt es bei der Verwendung einer Klasse ein Leistungsproblem? Wenn nicht, werfen Sie eine Münze, um zu entscheiden, welche Sie verwenden möchten.
Greg Burghardt
4
@Matthew: Wirklich wichtig ist, dass Sie die mühsame Logik der Normalisierung der Codes und Vergleiche zentralisieren. Danach ist "class versus struct" nur noch eine akademische Diskussion, bis Sie eine ausreichend große Auswirkung auf die Leistung messen, um die zusätzliche Entwicklerzeit für eine akademische Diskussion zu rechtfertigen.
Greg Burghardt
1
Das stimmt, es macht mir nichts aus, von Zeit zu Zeit eine akademische Diskussion zu führen, wenn es mir hilft, in Zukunft besser informierte Lösungen zu finden.
Matthew
@ Matthew: Yup, du hast absolut recht. Sie sagen: "Reden ist billig." Es ist sicherlich billiger als nicht zu reden und etwas schlecht zu bauen. :)
Greg Burghardt
13

Verwenden Sie das Flyweight-Muster

Da der Flughafen zu Recht unveränderlich ist, ist es nicht erforderlich, mehr als eine Instanz einer bestimmten Instanz zu erstellen, z. B. SFO. Verwenden Sie eine Hashtabelle oder ähnliches (Anmerkung: Ich bin ein Java-Typ, nicht C #, daher können die genauen Details variieren), um Flughäfen beim Erstellen zwischenzuspeichern. Bevor Sie eine neue erstellen, checken Sie die Hashtabelle ein. Sie geben niemals Flughäfen frei, daher muss GC sie nicht freigeben.

Ein weiterer kleiner Vorteil (zumindest in Java, bei C # nicht sicher) ist, dass Sie keine equals()Methode schreiben müssen , ein einfacher ==Wille genügt. Gleiches gilt für hashcode().

user949300
quelle
3
Hervorragende Nutzung des Flyweight-Musters.
Neil
2
Vorausgesetzt, OP verwendet weiterhin eine Struktur und keine Klasse. Behandelt die String-Internierung nicht bereits die wiederverwendbaren String-Werte? Die Strukturen befinden sich bereits auf dem Stapel, die Zeichenfolgen werden bereits wiederverwendet, um doppelte Werte im Speicher zu vermeiden. Welchen zusätzlichen Nutzen würde das Fliegengewicht bringen?
Flater
Etwas, auf das man achten muss. Wenn ein Flughafen hinzugefügt oder entfernt wird, sollten Sie diese statische Liste aktualisieren, ohne die Anwendung neu zu starten oder erneut bereitzustellen. Flughäfen werden nicht oft hinzugefügt oder entfernt, aber Geschäftsinhaber sind in der Regel ein wenig verärgert, wenn eine einfache Änderung so kompliziert wird. "Kann ich es nicht einfach irgendwo hinzufügen ?! Warum müssen wir einen Release- / Anwendungsneustart einplanen und unseren Kunden Unannehmlichkeiten bereiten?" Zuerst wollte ich aber auch einen statischen Cache verwenden.
Greg Burghardt
@ Flater Angemessener Punkt. Ich würde sagen, dass weniger Junior-Programmierer über Stack vs. Heap nachdenken müssen. Siehe auch meinen Zusatz - es ist nicht erforderlich, equals () zu schreiben.
user949300
1
@Greg Burghardt Wenn der getAirportOrCreate()Code richtig synchronisiert ist, gibt es keinen technischen Grund, warum Sie zur Laufzeit keine neuen Flughäfen nach Bedarf erstellen können. Es kann geschäftliche Gründe geben.
user949300
3

Ich bin kein besonders fortgeschrittener Programmierer, aber wäre das nicht eine perfekte Verwendung für ein Enum?

Es gibt verschiedene Möglichkeiten, Enum-Klassen aus Listen oder Zeichenfolgen zu erstellen. Hier ist eine, die ich in der Vergangenheit gesehen habe, nicht sicher, ob es der beste Weg ist.

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

Adam B
quelle
2
Wenn es möglicherweise Tausende verschiedener Werte gibt (wie dies bei Flughafencodes der Fall ist), ist eine Aufzählung einfach nicht praktikabel.
Ben Cottrell
Ja, aber der Link, den ich gepostet habe, ist, wie man Strings als Enums lädt. Hier ist ein weiterer Link zum Laden einer Nachschlagetabelle als Aufzählung. Es könnte ein bisschen Arbeit sein, würde aber die Macht der Aufzählungen ausnutzen. exceptionnotfound.net/…
Adam B
1
Oder es kann eine Liste gültiger Codes aus einer Datenbank oder Datei geladen werden. Dann wird ein Flughafencode überprüft, um zu dieser Liste zu gehören. Dies ist das, was Sie normalerweise tun, wenn Sie die Werte nicht mehr fest codieren möchten und / oder die Liste zu lang wird, um sie zu verwalten.
Neil
@BenCottrell das ist genau das, wofür Code-Gen und T4-Templates sind.
RubberDuck
3

Einer der Gründe, warum Sie mehr GC-Aktivitäten sehen, ist, dass Sie jetzt eine zweite Zeichenfolge erstellen - die .ToUpperInvariant()Version der ursprünglichen Zeichenfolge. Die ursprüngliche Zeichenfolge kann unmittelbar nach der Ausführung des Konstruktors für GC verwendet werden, und die zweite Zeichenfolge kann gleichzeitig mit dem AirportObjekt verwendet werden. Möglicherweise können Sie es auf andere Weise minimieren (beachten Sie den dritten Parameter für 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
quelle
Ergibt dies nicht unterschiedliche Hash-Codes für gleiche (aber unterschiedlich großgeschriebene) Flughäfen?
Hero Wanders
Ja, das würde ich mir vorstellen. Dangit.
Jesse C. Slicer
Dies ist ein sehr guter Punkt, an den ich nie gedacht habe. Ich werde versuchen, diese Änderungen vorzunehmen.
Matthew
1
In Bezug auf GetHashCodesollte nur verwenden StringComparer.OrdinalIgnoreCase.GetHashCode(Code)oder ähnlich
Matthew