2009-05-13 5 views
2

Ich habe schon vor einiger Zeit einige Schulklassen genommen und um ehrlich zu sein habe ich nie wirklich das Konzept der Klassen verstanden. Ich bin vor kurzem "zurück auf das Pferd" gegangen und habe versucht, eine reale Anwendung für die Erstellung einer Klasse zu finden.Bitte kritisieren Sie meine Klasse

Sie gesehen haben, dass ich versuche eine Menge Stammbaum-Daten zu analysieren, die in einem sehr alten und veralteten Format genannt gedcom

ich eine Gedcom Reader-Klasse erstellt in der Datei, Prozess zu lesen und machen Sie es als zwei Listen verfügbar, die die Daten enthalten, die ich benötigte, um zu verwenden

Wichtiger zu mir ist, schuf ich eine Klasse, es zu tun, also würde ich sehr gerne die Experten hier bekommen, um mir zu sagen, was ich tat richtig und was hätte ich besser machen können (ich werde nicht falsch sagen, weil das Ding funktioniert und das ist gut genug für mich)

Klasse:

using System; 
using System.Collections.Generic; 
using System.Text; 
using System.IO; 

namespace GedcomReader 
{ 

    class Gedcom 
    { 
     private string GedcomText = ""; 


     public struct INDI 
     { 
      public string ID; 
      public string Name; 
      public string Sex; 
      public string BDay; 
      public bool Dead; 
     } 
     public struct FAM 
     { 
      public string FamID; 
      public string Type; 
      public string IndiID; 
     } 

     public List<INDI> Individuals = new List<INDI>(); 
     public List<FAM> Families = new List<FAM>(); 


     public Gedcom(string fileName) 
     { 
      using (StreamReader SR = new StreamReader(fileName)) 
      { 
       GedcomText = SR.ReadToEnd(); 
      } 
      ReadGedcom(); 

     } 

     private void ReadGedcom() 
     { 
      string[] Nodes = GedcomText.Replace("0 @", "\u0646").Split('\u0646'); 
      foreach (string Node in Nodes) 
      { 
       string[] SubNode = Node.Replace("\r\n", "\r").Split('\r'); 
       if (SubNode[0].Contains("INDI")) 
       { 
        Individuals.Add(ExtractINDI(SubNode)); 

       } 
       else if (SubNode[0].Contains("FAM")) 
       { 
        Families.Add(ExtractFAM(SubNode)); 
       } 
      } 
     } 

     private FAM ExtractFAM(string[] Node) 
     { 
      string sFID = Node[0].Replace("@ FAM", ""); 
      string sID = ""; 
      string sType = ""; 
      foreach (string Line in Node) 
      { 
       // If node is HUSB 
       if (Line.Contains("1 HUSB ")) 
       { 
        sType = "PAR"; 
        sID = Line.Replace("1 HUSB ", "").Replace("@", "").Trim(); 
       } 
       //If node for Wife 
       else if (Line.Contains("1 WIFE ")) 
       { 
        sType = "PAR"; 
        sID = Line.Replace("1 WIFE ", "").Replace("@", "").Trim(); 
       } 
       //if node for multi children 
       else if (Line.Contains("1 CHIL ")) 
       { 
        sType = "CHIL"; 
        sID = Line.Replace("1 CHIL ", "").Replace("@", ""); 
       } 
      } 
      FAM Fam = new FAM(); 
      Fam.FamID = sFID; 
      Fam.Type = sType; 
      Fam.IndiID = sID; 

      return Fam; 
     } 
     private INDI ExtractINDI(string[] Node) 
     { 

      //If a individual is found 
      INDI I = new INDI(); 
      if (Node[0].Contains("INDI")) 
      { 
       //Create new Structure 

       //Add the ID number and remove extra formating 
       I.ID = Node[0].Replace("@", "").Replace(" INDI", "").Trim(); 
       //Find the name remove extra formating for last name 
       I.Name = Node[FindIndexinArray(Node, "NAME")].Replace("1 NAME", "").Replace("/", "").Trim(); 
       //Find Sex and remove extra formating 
       I.Sex = Node[FindIndexinArray(Node, "SEX")].Replace("1 SEX ", "").Trim(); 

       //Deterine if there is a brithday -1 means no 
       if (FindIndexinArray(Node, "1 BIRT ") != -1) 
       { 
        // add birthday to Struct 
        I.BDay = Node[FindIndexinArray(Node, "1 BIRT ") + 1].Replace("2 DATE ", "").Trim(); 
       } 

       // deterimin if there is a death tag will return -1 if not found 
       if (FindIndexinArray(Node, "1 DEAT ") != -1) 
       { 
        //convert Y or N to true or false (defaults to False so no need to change unless Y is found. 
        if (Node[FindIndexinArray(Node, "1 DEAT ")].Replace("1 DEAT ", "").Trim() == "Y") 
        { 
         //set death 
         I.Dead = true; 
        } 
       } 

      } 
      return I; 
     } 
     private int FindIndexinArray(string[] Arr, string search) 
     { 
      int Val = -1; 
      for (int i = 0; i < Arr.Length; i++) 
      { 
       if (Arr[i].Contains(search)) 
       { 
        Val = i; 
       } 
      } 
      return Val; 
     } 
    } 
} 

Umsetzung:

using System; 
using System.Windows.Forms; 
using GedcomReader; 

namespace WindowsFormsApplication1 
{ 
    public partial class Form1 : Form 
    { 
     public Form1() 
     { 
      InitializeComponent(); 
     } 

     private void button1_Click(object sender, EventArgs e) 
     { 
      string path = @"C:\mostrecent.ged"; 
      string outpath = @"C:\gedcom.txt"; 
      Gedcom GD = new Gedcom(path); 

      GraphvizWriter GVW = new GraphvizWriter("Family Tree"); 
      foreach(Gedcom.INDI I in GD.Individuals) 
      { 
       string color = "pink"; 
       if (I.Sex == "M") 
       { 
        color = "blue"; 
       } 
       GVW.ListNode(I.ID, I.Name, "filled", color, "circle"); 

       if (I.ID == "ind23800") 
       {MessageBox.Show("stop");} 
       //"ind23800" [ label="Sarah Mandley",shape="circle",style="filled",color="pink" ]; 
      } 
      foreach (Gedcom.FAM F in GD.Families) 
      { 

       if (F.Type == "par") 
       { 
        GVW.ConnNode(F.FamID, F.IndiID); 
       } 
       else if (F.Type =="chil") 
       { 
        GVW.ConnNode(F.IndiID, F.FamID); 
       } 
      } 
      string x = GVW.SB.ToString(); 
      GVW.SaveFile(outpath); 
      MessageBox.Show("done"); 
     } 
    } 

Ich bin besonders daran interessiert, wenn überhaupt etwas über die Strukturen getan werden könnte, ich weiß nicht, ob, wie ich sie bei der Umsetzung nutzen die größte ist aber wieder es funktioniert

Thanks a lot

+0

Danke für das feste Tag. Ich war mir sicher, dass ich es richtig gemacht hatte Google gab sogar meine Arbeit als Kritik zurück, aber ich schätze, du kannst Google einfach nicht für deine Rechtschreibprüfung vertrauen. – Crash893

+0

"Critique My Code" Fragen gehören auf codereview.se –

Antwort

4

Schnell Gedanken:

  • Verschachtelte Typen sollten nicht sichtbar sein.
  • ValueTypes (Strukturen) sollten unveränderlich sein.
  • Felder (Klassenvariablen) sollten nicht öffentlich sein. Expose sie stattdessen über Eigenschaften.
  • Überprüfen Sie die übergebenen Argumente auf ungültige Werte wie null.
+0

... Expose sie über Eigenschaften ... Besser noch, anstatt die Daten aus der Klasse abrufen, fügen Sie Funktionen der Klasse, um sie zu manipulieren, anstatt extern. Sag, frag nicht. –

+0

@ kenneth und Simon. Ich bin nicht mit den Begriffen vertraut. Hast du ein Beispiel? – Crash893

3

Ich schlage vor, Sie überprüfen diese Stelle aus: http://refactormycode.com/.

Für einige schnelle Dinge, Ihre Namensgebung ist das größte, was ich anfangen würde zu ändern. Keine Notwendigkeit, ALL-CAPS oder abgekürzte Begriffe zu verwenden.

Auch FxCop wird mit vielen vorgeschlagenen Änderungen helfen. Zum Beispiel würde FindIndexinArrayFindIndexInArray genannt werden.

EDIT:

Ich weiß nicht, ob dies ein Fehler im Code oder durch-Design, aber in FindIndexinArray, die Sie nicht aus der Schleife brechen, wenn Sie eine Übereinstimmung zu finden. Möchten Sie die erste (Pause) oder letzte (keine Pause) Übereinstimmung im Array?

+0

Ja, ich bin immer noch in der Fresse darüber, eine Person kann mehr als eine Frau oder mehr als ein Kind haben, so dass ich vielleicht die Indizes aller Kumpels oder alle Kinder im Moment nur tun, die erste, die es findet – Crash893

+0

Ich bin nicht genau sicher, was refactormycode.com tut, könntest du erklären – Crash893

+0

Nun, im Grunde, was Sie hier fragen: Sie schreiben Code für Leute, um Refaktor zu helfen - dh hoffentlich zu verbessern. –

4

Es könnte besser lesbar sein. Es ist schwer zu lesen und zu verstehen.

können Sie SOLID Prinzipien studieren (http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod)

Robert C.Martin gab gute Präsentation auf Oredev 2008 über sauberen Code (http://www.oredev.org/topmenu/video/agile/robertcmartincleancodeiiifunctions.4.5a2d30d411ee6ffd2888000779.html)

Einige Recomended Bücher über die Lesbarkeit des Codes zu lesen:

  • Kent Beck "Implemetierung Muster"

  • Robert C Martin „Clean Code "Robert C

  • Martin "Agile Prinzipien, Patterns und Praktiken in C#"

+1

+1 für die Erwähnung der SOLID-Prinzipien. –

Verwandte Themen