2017-09-07 3 views
0

Ich entwickle eine C# -Bibliothek mit .NET Framework 4.7 und Visual Studio 2017 Community.Prinzip der einfachen Verantwortung: Methode DoDatabaseBackup macht eine Menge Dinge

Ich versuche zu verstehen und Single-Verantwortung-Prinzip richtig zu verwenden.

ich diese Klasse habe eine Sicherung in einem SQL Server 2012-Datenbank zu tun:

public static class DbManagement 
{ 
    private static string sqlBackupStatement = 
     "BACKUP DATABASE [{0}] TO DISK = N'{1}' WITH COMPRESSION, COPY_ONLY, NAME=N'{2}'"; 

    /// <summary> 
    /// Do backup for a database. 
    /// </summary> 
    /// <param name="connectionString">Connection's string to database</param> 
    /// <param name="path">Path to safe the database</param> 
    /// <param name="backupFileName">Backup's file name</param> 
    /// <param name="backupName">This name will be used to identify this backup.</param> 
    /// <returns>Database's execution result</returns> 
    public static int DoDatabaseBackup(
     string connectionString, 
     string path, 
     string backupFileName, 
     string backupName) 
    { 
     if (string.IsNullOrWhiteSpace(connectionString)) 
      throw new ArgumentNullException(nameof(connectionString)); 
     if (string.IsNullOrWhiteSpace(path)) 
      throw new ArgumentNullException(nameof(path)); 
     if (string.IsNullOrWhiteSpace(backupFileName)) 
      throw new ArgumentNullException(nameof(backupFileName)); 
     if (string.IsNullOrWhiteSpace(backupName)) 
      throw new ArgumentNullException(nameof(backupName)); 

     string databaseName = GetDatabaseName(connectionString); 
     string fullPath = GetFullPath(path); 
     string pathWithFile = fullPath + backupFileName + "_" + DateTime.Now.ToString("yyyyMMddHHmm") + ".bak"; 
     string description = string.Format("{0} - Full database Backup", backupName); 

     if (!Directory.Exists(fullPath)) 
      Directory.CreateDirectory(fullPath); 

     string sqlStatement = string.Format(sqlBackupStatement, databaseName, pathWithFile, description); 

     TRZLDbContext context = new TRZLDbContext(connectionString); 

     return 
      context.Database.ExecuteSqlCommand(TransactionalBehavior.DoNotEnsureTransaction, sqlStatement); 
    } 

    private static string GetDatabaseName(string connectionString) 
    { 
     SqlConnectionStringBuilder sqlConnBuilder = 
      new SqlConnectionStringBuilder(connectionString); 

     return sqlConnBuilder.InitialCatalog; 
    } 

    private static string GetFullPath(string path) 
    { 
     StringBuilder builder = new StringBuilder(path); 
     if (!path.EndsWith("\\")) 
      builder.Append("\\"); 

     DateTime today = DateTime.Now; 

     builder.Append(today.Year.ToString()); 
     builder.Append("\\"); 
     builder.Append(today.Month.ToString()); 
     builder.Append("\\"); 

     return builder.ToString(); 
    } 
} 

Ich denke, ein DoDatabaseBackup eine Menge Dinge in Methode tun:

  1. Get Datenbanknamen.
  2. Vollständiger Pfad zum Speichern der Sicherungsdatei.
  3. Den vollständigen Pfad mit dem Dateinamen des Backups abrufen.
  4. Backup-Beschreibung erstellen.
  5. Erstellen Sie ein Verzeichnis, in dem ich die Sicherung speichern werde, wenn sie nicht existiert.
  6. Erstellen Sie die SQL-Anweisung, um die Sicherung durchzuführen.
  7. Und schließlich ... tun Sie das Backup.

Bin ich Single-Verantwortung-Prinzip korrekt hier verwenden? Es ist nicht, muss ich Punkte 1 bis 6 zu 6 Methoden bewegen?

Antwort

-1

Beide.

Erstellen Sie keine Dinge wie den Backup-Namen im laufenden Betrieb neu. Habe eine Methode/Funktion/was auch immer dir das gibt.

Andernfalls ist es für eine Funktion in Ordnung, mehr als eine Sache zu tun, um ein Ziel zu erreichen, das diese Dinge subsumiert. Nichtsdestotrotz duplizieren Sie keine an anderer Stelle gefundenen Aktionen (z. B. das Erstellen eines Backup-Pfads).

Das Prinzip der einheitlichen Verantwortung besteht darin, eindeutige Objekte an einzelnen Stellen zu modularisieren - es ist eine Verallgemeinerung der Ein-Definition-Regel. (Ja, ich weiß, dass Sie Java verwenden. Sorry ... LOL)

+0

Danke für Ihre Antwort, Oh, ich habe einen Tippfehler. Die erste Frage lautet: Benutze ich das Prinzip der einfachen Verantwortung ** richtig ** hier? Übrigens benutze ich kein Java, ich benutze C#. – VansFannel

+1

Ich stimme nicht zu, die SRP besagt, dass eine Klasse oder eine Methode nur aus einem Grund geändert werden sollte. In diesem Fall hat die Methode viele Verantwortlichkeiten, die für mich klar gegen das Prinzip verstoßen – Mehdi

0

Sie könnten dies in eine Reihe von separaten Klassen mit unterschiedlichen Verantwortlichkeiten umgestalten. Wenn Sie denken, die Typen von Objekten, die hier dargestellt werden könnten, würden Sie etwas wie:

  • Eine Verbindungszeichenfolge
  • Ein Datenbank-Backup
  • Eine Backup-Anweisung
  • Ein Backup-Datei Name

Bei SRP ist es wichtig darüber nachzudenken, wie sich Objekte ändern können. Zum Beispiel kann sich der Algorithmus zum Ändern des Sicherungsdateinamens ändern, so dass er möglicherweise in einer separaten Klasse dargestellt werden soll, so dass er bei Bedarf für einen neuen Algorithmus ausgetauscht werden kann. Möglicherweise müssen Sie die SQL-Anweisung ändern, wenn Sie einen anderen Dateityp verwenden. Die Verbindungszeichenfolge hat auch eine Abhängigkeit von SQL, die sich in Zukunft ändern kann.

hatte ich einen gehen auf Ihre obige Methode Refactoring und kam mit den folgenden:

public class ConnectionString 
{ 
    private readonly string connectionString; 

    public ConnectionString(string connectionString) 
    { 
     this.connectionString = connectionString; 
    } 

    public string DatabaseName() 
    { 
     SqlConnectionStringBuilder sqlConnBuilder = 
      new SqlConnectionStringBuilder(this.connectionString); 

     return sqlConnBuilder.InitialCatalog; 
    } 
} 

public class BackupFileName 
{ 
    private readonly string backupFilePath; 
    private readonly string backupFileName; 

    public BackupFileName(string backupFilePath, string backupFileName) 
    { 
     this.backupFilePath = backupFilePath; 
     this.backupFileName = backupFileName; 
    } 

    public string FullPath => this.backupFilePath; 

    public override string ToString() 
    { 
     return this.GetFullPath(this.backupFilePath) + this.backupFileName + "_" + 
       DateTime.Now.ToString("yyyyMMddHHmm") + ".bak"; 
    } 

    private string GetFullPath(string path) 
    { 
     StringBuilder builder = new StringBuilder(path); 
     if (!path.EndsWith("\\")) 
      builder.Append("\\"); 

     DateTime today = DateTime.Now; 

     builder.Append(today.Year.ToString()); 
     builder.Append("\\"); 
     builder.Append(today.Month.ToString()); 
     builder.Append("\\"); 

     return builder.ToString(); 
    } 
} 

public class BackupStatement 
{ 
    private string sqlBackupStatement = 
     "BACKUP DATABASE [{0}] TO DISK = N'{1}' WITH COMPRESSION, COPY_ONLY, NAME=N'{2}'"; 

    private readonly string databaseName; 
    private readonly string filePath; 
    private readonly string description; 

    public BackupStatement(string databaseName, string filePath, string description) 
    { 
     this.databaseName = databaseName; 
     this.filePath = filePath; 
     this.description = description; 
    } 

    public override string ToString() 
    { 
     return string.Format(sqlBackupStatement, databaseName, filePath, description); 
    } 
} 

public class DatabaseBackup 
{ 
    private readonly ConnectionString connectionString; 
    private readonly string backupName; 
    private readonly BackupFileName backupFileName; 

    public DatabaseBackup(
     string connectionString, 
     string path, 
     string backupFileName, 
     string backupName) 
    { 
     if (string.IsNullOrWhiteSpace(connectionString)) 
      throw new ArgumentNullException(nameof(connectionString)); 
     if (string.IsNullOrWhiteSpace(path)) 
      throw new ArgumentNullException(nameof(path)); 
     if (string.IsNullOrWhiteSpace(backupFileName)) 
      throw new ArgumentNullException(nameof(backupFileName)); 
     if (string.IsNullOrWhiteSpace(backupName)) 
      throw new ArgumentNullException(nameof(backupName)); 

     this.connectionString = new ConnectionString(connectionString); 
     this.backupFileName = new BackupFileName(path, backupFileName); 
     this.backupName = backupName; 
    } 

    /// <summary> 
    /// Creates a backup for a database. 
    /// </summary> 
    /// <returns>Database's execution result</returns> 
    public int Create() 
    { 
     string description = string.Format("{0} - Full database Backup", this.backupName); 

     if (!Directory.Exists(this.backupFileName.FullPath)) 
      Directory.CreateDirectory(this.backupFileName.FullPath); 

     TRZLDbContext context = new TRZLDbContext(this.connectionString.ToString()); 

     return 
      context.Database.ExecuteSqlCommand(
       TransactionalBehavior.DoNotEnsureTransaction, 
       new BackupStatement(
        this.connectionString.DatabaseName(), 
        this.backupFileName.ToString(), 
        description).ToString()); 
    } 
} 

Es ist nicht perfekt, und jeder der oben genannten Klassen sollten wirklich Schnittstellen werden (wie pro Dependency Inversion), aber ich hoffe es zeigt einen Punkt.

Verwandte Themen