C# vereinfachen



  • Hi.
    Kann mir jmd sagen wie ich folgenden Code vereinfachen kann?
    Ich mein, ich mach ja theoretisch 5mal das gleiche, nur dass ich jedesmal ne andere Variable hab.
    Aber man muss des doch irgendwie vereinfachen können.

    foreach (XmlNode node in nodeListAktiv) 
    { 
        if (node.InnerText.ToString() == "true") 
            listcheck_aktiv.Add(true); 
        else 
            listcheck_aktiv.Add(false); 
    } 
    
    foreach (XmlNode node in nodeListInterop) 
    { 
        if (node.InnerText.ToString() == "true") 
            listcheck_interop.Add(true); 
        else 
            listcheck_interop.Add(false); 
    } 
    
    foreach (XmlNode node in nodeListWindows) 
    { 
        if (node.InnerText.ToString() == "true") 
            listcheck_windows.Add(true); 
        else 
            listcheck_windows.Add(false); 
    } 
    
    foreach (XmlNode node in nodeListSelfreg) 
    { 
        if (node.InnerText.ToString() == "true") 
            listcheck_selfreg.Add(true); 
        else 
            listcheck_selfreg.Add(false); 
    } 
    
    foreach (XmlNode node in nodeListDatev) 
    { 
        if (node.InnerText.ToString() == "true") 
            listcheck_datev.Add(true);
    


  • Pack deine 5 List<XMLNode> in ein Array und deine 5 List<bool> in ein Array und dann kannst du einfach sowas schreiben:

    List<XmlNode>[] Nodes = new List<XmlNode>[5];
    // Hinzufügen der Listen
    .
    .
    .
    List<bool>[] Bools = new List<bool>[5];
    // Hinzufügen der Listen
    .
    .
    .
    for (int i = 0; i < 5; i++) {
      foreach (XmlNode node in Nodes[i]) {
        Bools.Add(node.InnerText == "true");
      }
    }
    


  • Alternativ gehts auch mit ner Hashtable, was imho den Vorteil hat das man sich nicht den Index merken muß, sondern auf die bool-listen so zugreifen kann:

    (List<bool)listchecks[nodeListDatev]

    Hashtable listchecks = new Hashtable();
    
    listchecks[nodeListAktiv] = new List<bool>();
    listchecks[nodeListInterop] = new List<bool>();
    listchecks[nodeListWindows] = new List<bool>();
    listchecks[nodeListSelfreg] = new List<bool>();
    listchecks[nodeListDatev] = new List<bool>();
    
    foreach(XmlNodeList nodelist in new XmlNodeList[] { nodeListAktiv, nodeListInterop, nodeListWindows, nodeListSelfreg, nodeListDatev } )
    {
       foreach (XmlNode node in nodeListAktiv) 
       { 
          ((List<bool>)listchecks[nodelist]).Add((node.InnerText.ToString() == "true")); 
       } 
    }
    


  • ups, korrektur, die innere foreach muss heissen:

    foreach (XmlNode node in nodelist)

    und nicht

    foreach (XmlNode node in nodeListAktiv)



  • Sorry, das ist doch alles Quatsch.

    Man extrahiert den Körper der Schleife in eine eigene Methode und ruft diese auf:

    void AddCheck(XmlNode node) {
        listcheck_aktiv.Add(node.InnerText.ToString() == "true");
    }
    
    // …
    
    nodeListAktiv.ForEach(AddCheck);
    nodeListInterop.ForEach(AddCheck);
    nodeListWindows.ForEach(AddCheck);
    nodeListSelfreg.ForEach(AddCheck);
    nodeListDatev.ForEach(AddCheck);
    


  • Konrad Rudolph schrieb:

    Sorry, das ist doch alles Quatsch.

    Man extrahiert den Körper der Schleife in eine eigene Methode und ruft diese auf:

    void AddCheck(XmlNode node) {
        listcheck_aktiv.Add(node.InnerText.ToString() == "true");
    }
    
    // …
    
    nodeListAktiv.ForEach(AddCheck);
    nodeListInterop.ForEach(AddCheck);
    nodeListWindows.ForEach(AddCheck);
    nodeListSelfreg.ForEach(AddCheck);
    nodeListDatev.ForEach(AddCheck);
    

    dabei übersiehst Du aber das jede nodelist auch eine eigene listcheck-liste hat.

    In Deinem Fall würden alle nodelisten in die gleiche listcheck-liste schreiben.



  • skals schrieb:

    Konrad Rudolph schrieb:

    Sorry, das ist doch alles Quatsch.

    Man extrahiert den Körper der Schleife in eine eigene Methode und ruft diese auf:

    void AddCheck(XmlNode node) {
        listcheck_aktiv.Add(node.InnerText.ToString() == "true");
    }
    
    // …
    
    nodeListAktiv.ForEach(AddCheck);
    nodeListInterop.ForEach(AddCheck);
    nodeListWindows.ForEach(AddCheck);
    nodeListSelfreg.ForEach(AddCheck);
    nodeListDatev.ForEach(AddCheck);
    

    dabei übersiehst Du aber das jede nodelist auch eine eigene listcheck-liste hat.

    In Deinem Fall würden alle nodelisten in die gleiche listcheck-liste schreiben.

    Na gut, dann passt man die Funktionssignatur halt an und arbeitet mit einer anonymen Methode, um den ForEach-Aufruf anzupassen – oder man schenkt sich das, flucht, weil C# keine Parameter-Binder für Funktionen höherer Ordnung kennt, und verwendet eine konventionelle 'foreach'-Schleife.

    Aber die Auslagerung in eine Methode sollte man in jedem Fall verwenden.



  • Konrad Rudolph schrieb:

    Aber die Auslagerung in eine Methode sollte man in jedem Fall verwenden.

    Wieso? Sowas ist hier doch völlig unnötig. Die 5 Schleifen aus dem Anfangspost entsprechen doch schon logisch gesehen einer Funktion die halt die boolschen Werte in die Liste einträgt. Die Varianten von skals und mir vereinfachen jetzt diese 5 Schleifen auf eine verschachtelte und brauchen paar Zeilen Vorbereitung dafür. Wieso sollte man da noch was auftrennen? das würde dem Funktionsgedanken wiedersprechen, da die so entstehende Minifunktion im größeren Kontext eigentlich unnötig wäre.



  • Zwergli schrieb:

    Konrad Rudolph schrieb:

    Aber die Auslagerung in eine Methode sollte man in jedem Fall verwenden.

    Wieso?

    Weil ihr gegen eine konkrete Implementierung programmiert, ich stattdessen gegen eine Schnittstelle.

    Abgesehen davon, dass mein Code etwas kürzer (und, argumentierbar eleganter, da er über weniger Verschachtelungen verfügt) ist, lässt er sich leichter erweitern bzw. austauschen.

    Für diesen einfachen Fall ist das natürlich recht unwichtig, aber da kann man sich schnell verheddern. Es ist einfach eine gute Regel, die man immer berüchsichtigen sollte, unterschiedliche Logiken soweit wie möglich voneinander zu trennen.



  • Konrad Rudolph schrieb:

    Zwergli schrieb:

    Konrad Rudolph schrieb:

    Aber die Auslagerung in eine Methode sollte man in jedem Fall verwenden.

    Wieso?

    Weil ihr gegen eine konkrete Implementierung programmiert, ich stattdessen gegen eine Schnittstelle.

    Abgesehen davon, dass mein Code etwas kürzer (und, argumentierbar eleganter, da er über weniger Verschachtelungen verfügt) ist, lässt er sich leichter erweitern bzw. austauschen.

    Für diesen einfachen Fall ist das natürlich recht unwichtig, aber da kann man sich schnell verheddern. Es ist einfach eine gute Regel, die man immer berüchsichtigen sollte, unterschiedliche Logiken soweit wie möglich voneinander zu trennen.

    Versteh mich nciht falsch, ich stimme Dir in allen Punkten zu was die vorn Dir genannten Vorteile Deines Ansatzes angeht. Nur leider ist er im konkreten Fall _falsch_ weil er nicht der geforderten Funktionalität des OP entspricht. Mit Deinem Ansatz müßtest Du fünf verschiedene AddCheck-Funktionen schreiben, halt eine für jede Checkliste. Und damit wärest Du wieder beim Ausgangsprunkt des Problems, Copy-paste code...



  • skals schrieb:

    Nur leider ist er im konkreten Fall _falsch_ weil er nicht der geforderten Funktionalität des OP entspricht. Mit Deinem Ansatz müßtest Du fünf verschiedene AddCheck-Funktionen schreiben, halt eine für jede Checkliste.

    Nö, müsste man nicht, ich sagte doch schon: Die Liste kann man als zusätzlichen Parameter übergeben. Aber Du hast in einer Hinsicht recht: Ich schreibe in jedem Fall fünf Schleifen, das ist schlecht. D.h. man müsste trotdem euer Mapping verwenden, das hatte ich übersehen.


Anmelden zum Antworten