Laden...

Code vereinfachen und NullReferenceException

Letzter Beitrag vor einem Jahr 8 Posts 744 Views
Code vereinfachen und NullReferenceException

Folgender Code kommt mir etwas lang vor. Kann man den sinnvoll kürzen?
Außerdem gibt es in Zeile 9 eine NullReferenceException.
Ist auch richtig, denn auf meinem privaten System gibt es den Registry-Eintrag nicht. Aber ich prüfe doch auf "null"? VS sagt aber auch, dass ich nicht prüfe (grün unterstrichen), also prüfe ich offenbar falsch.

Hinzu kommt, dass der gesuchte Schlüssel ein dword ist und keine Zeichenkette. Deswegen muss ich offenbar eine "doppelte Umwandlung" (so nenne ich das mal) vornehmen. Verstehe also nicht, was "(uint)(int)" genau machen.


public static string GetSystemIpv6State()
{
    string SystemIpv6State = "";
    try
    {
        using RegistryKey? ipv6 = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64).OpenSubKey(@"SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters");
        if (ipv6 != null)
        {
            object ipv6key = (uint)(int)ipv6.GetValue("DisabledComponents");
            if (ipv6key != null)
            {
                if (ipv6key.ToString() == "4294967295" || ipv6key.ToString() == "255")
                {
                    SystemIpv6State = "Deaktiviert";
                }
                else
                {
                    SystemIpv6State = "Aktiviert";
                }
            }
        }
        else
        {
            SystemIpv6State = "Undefinierter Zustand";
        }
    }
    catch (Exception)
    {
        SystemIpv6State = "Fehler";
    }
    return SystemIpv6State;
}

IDE: Visual Studio 2022
Sofern nicht anders genannt basieren meine Projekte auf C# und .net 6

Hi,

hast du mal im Debugger per Breakpoint geschaut, was ipv6key zurückgibt?

Ist das wirklich NULL? Wenn nicht, dann prüfe den Rückgabewert und behandle das entsprechend.
Ich würde darauf tippen, dass es daran liegt, dass uint und int niemals NULL sein werden - das lassen die Typen nicht zu.
Dafür gibt es IIRC uint? und int?.

Denke, das sollte dein Problem schon mal lösen was die Exception angeht.

Der Code sieht in der Tat alles andere als schön aus.
Anstelle eines String sollte man hier einen Status Code liefern.
Kann im einfachsten Fall ein int oder sauberer ein Enum sein.

Ebenfalls würde ich ddie Blöcke aufbrechnen und wenigere Vertierfungen umsetzen, was den Code auch lesbar macht.

Mein ungeprüfter Ansatz wäre erstmal billig folgender:


public static int GetSystemIpv6State()
{
	using RegistryKey ipv6 = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Registry64).OpenSubKey(@"SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters");
        
	// Fehler Code liefern
	if (ipv6 == null)
		return -1;
		
	object ipv6key = ipv6.GetValue("DisabledComponents");
		
	if (ipv6key == null)
		return -2;
	
	uint ipv6Value = (uint)ipv6key;
	
	// Nicht aktiv
	if (ipv6Value == 4294967295 || ipv6Value == 255)
		return 0;

	// Ist aktiv!
	return 1;
}

Hab den Ansatz mal kurz getestet.
DisabledComponents gibt es bei mir aber nicht obwohl IPv6 akiv ist.

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

Generell macht man niemals Fehlerbehandlung über Strings; in quasi keiner Sprache.
Auch gibts etwas modernere Ansätze als int, was im C-Bereich weit verbreitet ist.

  1. In C# ist das einfachste ein Enum:

public enum SystemIPv6State
{
  Unknown,
  Disabled,
  Inactive,
  Active
}

public static SystemIPv6State GetSystemIpv6State()

  1. Oder man baut sich eine Klasse, welche dann mehr Infos haben kann.
    Dieser Ansatz ist wohl am weitesten verbreitet. In ASP.NET exzessive verwendet, zB für Results: SignInResult
    Damit versucht man vor allem Exceptions zu vermeiden, die aus Runtime-sicht einfach teuer sind - und weil man eben Applikationslogik niemals über Exceptions steuern sollte.

  2. In anderen Sprachen, zunehmend aber auch in C#, gibt es dafür Discriminated Unions - zum Beispiel in F# ein absoluter Alltagsstandard.

Für C# gibts eine sehr gute Lib: GitHub - mcintyre321/OneOf: Easy to use F#-like discriminated unions for C# with exhaustive compile time matching
Verwenden wir hier im Forum auch (und ich auch in Alltagskundenprojekten).

Außerdem gibt es in Zeile 9 eine NullReferenceException.

Du castest hart; macht man nicht. wenn man nicht muss. Auf Null prüfen ist das eine, aber Du musst vorher prüfen, ob das casting überhaupt klappen kann.
Dafür gibts eigene operatoren bzw Keywords.

Siehe mein Beitrag in [FAQ] Casten aber richtig: Boxing/Unboxing - () / is / as / Pattern Matching
Ein hartes Casten wie


uint ipv6Value = (uint)ipv6key;

ist immer riskant und fast alle Fälle lassen sich vermeiden.
Geht viel sicherer mit


if(ipv6key is uint ipv6Value )
{  
    // ...
}

Boxing kann halt immer knallen. Da hilft nur die richtigen Operatoren wie is/as zu verwenden.

Ansonsten der generelle Verweis auf die Basics: Exception Handling - C# Programming Guide

Hi,

hast du mal im Debugger per Breakpoint geschaut, was ipv6key zurückgibt?

Ist das wirklich NULL?

Ja, ist "null". (Siehe Anhang) Ich würde es gerne verstehen, unabhängig davon, dass der restliche Code, bzw. Aufbau nicht schön ist. Direkt falsch ist es ja auch nicht.

IDE: Visual Studio 2022
Sofern nicht anders genannt basieren meine Projekte auf C# und .net 6

Du bedienst hier den Debugger falsch. Wie man am Debugger Fenster erkennen kann, knallt es vor der Zuweisung. Daher wird da immer null rauskommen.
Du musst den Wert auf null abfragen, nicht das Cast Resultat.


object ipv6key = ipv6.GetValue("DisabledComponents")
// ipv6key hier null?

Aber wenn Du hier das Casting richtig anwendest (zB mit is), dann ist das ohnehin nicht notwendig.

Und du bekommst auch den Hinweis von GetValue, dass null geliefert wird wenn der Key nicht vorhanden ist.
Entsprechend musst du auch einen Null Check vor dem Casting machen.
Null kann nicht in einen Wertetypen gecastet werden, was zu deiner Exception führt.

Doku:
RegistryKey.GetValue Methode (Microsoft.Win32)(system-string)

T-Virus

Developer, Developer, Developer, Developer....

99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.

Hallo

Ich hab mal eine Infrastruktur-Klasse erstellt die die oben behandelte Fragestellung erledigt.

Die Klasse ist universell, sprich, kann auf jeden Registry-Key angewendet werden. Zum Spaß hab ich noch eine Funktion zugefügt, die über alle Einträge eines Keys läuft und die Werte dazu liefert. Die Klasse unterstützt Fluent-API und Funktionen höherer Ordnung und kann wie folgt genutzt werden: (ich hab statt IPV6 mal die Hardware abgefragt 😉

Listing Consumer:

const string
    IPV6    = @"SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters",
    IPCOMP  = "DisabledComponents",

    MYPC        = @"SYSTEM\HardwareConfig\Current",
    MAINBOARD   = @"BaseBoardManufacturer"
    ;

RegistryState
.ForKey  (MYPC)
.ValueOf (MAINBOARD, value => WriteLine (value))
.Values  ((@this, name) =>
{
    WriteLine ($"Key: {name} = {@this.ValueOf(name).ToString()}");
})
;

Listing Klasse:


using Microsoft.Win32;

namespace MyApp;

public sealed class RegistryState
{
    //-------------------------------------------------------------------------
    // Static Members
    //-------------------------------------------------------------------------

    public static RegistryState ForKey (string keyName) => new (keyName);
    //-------------------------------------------------------------------------

    #pragma warning disable CA1416
    private static RegistryKey readKey (string keyName)
    =>
        RegistryKey
        .OpenBaseKey (RegistryHive.LocalMachine, RegistryView.Registry64)
        .OpenSubKey  (keyName);
    //-------------------------------------------------------------------------
    // Instance Members
    //-------------------------------------------------------------------------

    public RegistryKey RegKey {get; private set;}
    //-------------------------------------------------------------------------

    private RegistryState (string keyName) => RegKey = readKey (keyName);
    //-------------------------------------------------------------------------

    #pragma warning disable CA1416
    public RegistryState Values (Action<RegistryState, string> emit)
    {
        iterate (RegKey?.GetValueNames(), emit);
        return this;
    }
    //-------------------------------------------------------------------------

    private void iterate (string[] items, Action<RegistryState, string> emit)
    {
        var enumerator = items?.GetEnumerator();

        while ( enumerator?.MoveNext() ?? false )
            emit (this, (string)enumerator.Current);
    }
    //-------------------------------------------------------------------------
    #pragma warning disable CA1416
    public object ValueOf (string name) => RegKey?.GetValue(name);

    #pragma warning disable CA1416
    public RegistryState ValueOf (string name, Action<object> emit)
    {
        emit ( ValueOf(name) );
        return this;
    }
    //-------------------------------------------------------------------------
}

Gruß,

Tom