Einfaches Design der ASP.NET C # -Klasse

2085
Tom Gullen

Hier ist eine Klasse, die ich entwerfe:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Data;
using System.Data.SqlClient;

namespace Artworking.classes
{

    // A group permission level
    public class PermissionGroup
    {
        public int ID;                  // Unique ID for this group
        public bool hasFullControl;     // Does this group have complete control
        public user creator;            // Who created this group
        public string groupName;        // Reference name for group

        // Constructor for when a permission group ID is passed
        public PermissionGroup(SqlConnection cn, int ID)
        {
            using (SqlCommand cmd = new SqlCommand("SELECT creatorUserID, group_name, fullControl FROM tblATPermissionGroups WHERE ID = " + ID, cn))
            {
                SqlDataReader rdr = cmd.ExecuteReader();
                if (rdr.Read())
                {
                    this.creator.ID = int.Parse(rdr["creatorUserID"].ToString());
                    this.groupName = rdr["group_name"].ToString();
                    this.hasFullControl = bool.Parse(rdr["fullControl"].ToString());
                }
                rdr.Close();
            }
        }
    }

}

Bin ich auf dem richtigen Weg? Bitte beachten Sie, dass ich die integrierte Authentifizierung nicht verwende, da sie für ein altes System abwärtskompatibel sein muss. Was ich gerade überprüfe, ist, dass ich diese Klassen richtig handle und die Daten korrekt einlade.

Antworten
11
Kommentare sind nicht schlecht, aber warum sollten Sie nicht die XML-Kommentare (http://msdn.microsoft.com/de-de/magazine/cc302121.aspx) verwenden und sie durch IntelliSense® zurückfließen lassen? Larry Smithmier vor 9 Jahren 0
Danke LArry, ich wusste nichts über XML-Kommentare, aber für mein Projekt wäre es übertrieben :) Tom Gullen vor 9 Jahren 0

5 Antworten auf die Frage

11
akmad

Es ist im Allgemeinen keine gute Idee, diese Art der Verarbeitung im .ctor durchzuführen. Verschieben Sie es in eine Load-Methode (oder einen ähnlichen Namen). Dies bedeutet jedoch, dass Ihr Objekt nach der Instanziierung nicht wirklich geladen wird. Dies ist ein anderes Problem. Als solches würde ich empfehlen, das Unternehmen Informationen zu trennen ( HasFullControl, Creator, GroupName, usw.) vom Typ eingeben, die Informationen aus der Datenbank lädt. Sie können den PermissionGroupTyp ändern, um nur die Dateneigenschaften zu erhalten, und dann die Ladelogik auf eine Datenschicht verschieben, die für das Erstellen der Instanz und das Laden der Daten verantwortlich ist.

Wenn Sie Text mit dem Ende einer SQL-Zeichenfolge verketten, können Sie SQL-Injection-Angriffen ausgesetzt werden (in diesem speziellen Fall jedoch nicht). Verwenden Sie stattdessen einen Parameter:

using (SqlCommand cmd = new SqlCommand("SELECT creatorUserID, group_name, 
    fullControl FROM tblATPermissionGroups WHERE ID = @id", cn))
{
    cmd.Parameters.AddWithValue("@id", id);
    ...
}

Sie müssen den SqlDataReaderin einen using-Block einwickeln .

Verwenden Sie öffentliche Readonly-Eigenschaften anstelle von öffentlichen Feldern.

Erwägen Sie die Verwendung von IDbConnectioneher als SqlConnection. Auf diese Weise können Sie den Datenaufruf beim Testen dieser Methode einfacher simulieren und die Unterstützung anderer RDBMS bei Bedarf erleichtern.

Werfen Sie hilfreiche Ausnahmen, wenn beim Füllen der Datenfelder etwas schief geht.

Befolgen Sie die .Net Framework-Namensrichtlinien

6
Anthony Pegram

Betrachten Sie die von C # akzeptierten Verfahren. Wie in anderen Antworten erwähnt, haben Sie Mitgliederfelder als öffentlich markiert. In C # stellen Sie Werte normalerweise als Eigenschaften bereit und halten Mitglieder privat. Beginnend mit C # 3 müssen Sie nicht einmal explizit Mitgliederfelder erstellen, um Ihre Eigenschaften zu sichern, es sei denn, Getter und Setter sind nicht trivial. Erwägen Sie daher, Ihre Felder durch zu ersetzen

public int ID { get; private set; }
public bool HasFullControl { get; private set; }

Beachten Sie, dass ich privatezum Setter hinzugefügt habe. Dabei wird davon ausgegangen, dass Sie nicht möchten, dass der Code außerhalb Ihrer Klasse diese Werte festlegt, wodurch Ihre Berechtigungen quasi überschrieben werden. Fühlen Sie sich frei, diesen Modifikator zu entfernen, wenn dies nicht der Fall ist.

Beachten Sie auch, dass C # -Praktiken vorschreiben, dass öffentlich sichtbare Namen im Pascal-Gehäuse anstelle Ihres Kamelgehäuses liegen. Schreibe das erste Zeichen deines Namespaces (einschließlich verschachtelter Namespaces), Klassen, Eigenschaften, Methoden usw. groß.

@akmad hat eine Reihe weiterer Bedenken angesprochen, etwa den Konstruktor, der kostspielige Arbeit leistet, und die Anfälligkeit für die SQL-Einspeisung im Allgemeinen (wenn auch nicht speziell hier, wie Akmad auch unterstreicht). Wenn Sie einen Datenbankaufruf ausführen müssen, um Ihr Objekt zu erstellen, sollten Sie einen werkseitigen Ansatz berücksichtigen.

public class PermissionGroup
{
    private PermissionGroup() { } // class cannot be constructed from outside

    public static PermissionGroup GetPermissionGroup(/* your parameters */)
    {
        // build the object starting here!
    }
}
0
Mark Loeser

Ich weiß nichts über C #, also ist dies nur ein Kommentar, der auf dem Design basiert. Gibt es einen Grund, warum diese Mitglieder nicht privat sind? Warum stellen Sie keine Zugriffsmethoden zur Verfügung oder verschieben (wie ich bereits in vielen Kommentaren auf dieser Website bereits gesagt habe) tatsächlich Verhalten in diese Klasse, anstatt jedes einzelne Datenmitglied freizulegen? Dies ist beispielsweise eine Berechtigungsgruppe, die über eine Funktion namens "isAuthorized" verfügt, an die Sie einen Befehl übergeben können, um herauszufinden, ob diese Gruppe das zulässt. Auf diese Weise können Sie die Berechtigungen möglicherweise weiter verfeinern, wenn Sie vorwärts gehen, und niemand muss wissen, dass diese Klasse verwendet wird.

0
winwaed

Ich stimme Mark zu: Ich würde die Mitglieder privat machen und ihnen dann Property Accessors hinzufügen. Sie können dann den Zugriff steuern, wenn Sie dies jetzt oder in der Zukunft benötigen.

Beispielsweise möchten Sie möglicherweise eines der Mitglieder schreibgeschützt machen. Einfach zu tun: Implementieren Sie die get () - Eigenschaft, aber nicht die set () - Eigenschaft.

In ähnlicher Weise kann die Datenbereichsüberprüfung leicht zum set () hinzugefügt werden.

0
NPehrsson

Ich würde die Leseoperation in ein Repository und ein Domänenobjekt aufteilen, das die Daten enthält. Ich würde auch einen Unit-Work-Handler in das Repository einschicken, der die DB-Verbindung und die Transaktion abwickelt

public class PermissionGroupRepository
{
    public PermissionGroupRepository(IUnitOfWork unitOfWork)
    { }
    public PermissionGroup GetPermissionGroup(/* your parameters */)
    { }
}


public class PermissionGroup
{
     /* Properties */
}