Kann diese API verbessert werden?

Question

In unserer CORE-Bibliothek bieten wir diese Klasse als eine Abstraktion von 20.000 Zeilen an. Können Sie etwas falsch mit der Art sehen, wie dies entworfen ist?

Hinweis1: Diese Klasse verfügt über eine SharpZipLib-Unterstützung.

Hinweis2: SharpZipLib ist ungefähr 20K Zeilen.

public static class Compression 
{ 
    public static Byte[] CompressBytes(Byte[] input); 
    public static Byte[] CompressBytes(Byte[] input, Format format); 
    public static Byte[] CompressBytes(Byte[] input, Format format, Level level); 

    public static Byte[] DecompressBytes(Byte[] input); 
    public static Byte[] DecompressBytes(Byte[] input, Format format); 

    public static String CompressString(String input); 
    public static String CompressString(String input, Format format); 
    public static String CompressString(String input, Format format, Level level); 

    public static String DecompressString(String input); 
    public static String DecompressString(String input, Format format); 

    public static void CompressFile(String input_file_path, String output_file_path); 
    public static void CompressFile(String input_file_path, String output_file_path, Format format); 
    public static void CompressFile(String input_file_path, String output_file_path, Format format, Level level); 

    public static void DecompressFile(String input_file_path, String output_file_path); 
    public static void DecompressFile(String input_file_path, String output_file_path, Format format); 

    public static void CompressFolder(String input_folder_path, String output_file_path); 
    public static void CompressFolder(String input_folder_path, String output_file_path, Format format); 
    public static void CompressFolder(String input_folder_path, String output_file_path, Format format, Level level); 

    public static void DecompressFolder(String input_file_path, String output_file_path); 
    public static void DecompressFolder(String input_file_path, String output_file_path, Format format); 
} 

Answer 1

Ich würde empfehlen, diese einzelne Klasse in mehrere Klassen zu brechen. Im Allgemeinen brechen statische Utility-Klassen viele Regeln, von denen nicht zuletzt die Separation of Concerns gilt. Obwohl ja alle Methoden in dieser Klasse mit Komprimierung umgehen, befassen sie sich mit der Komprimierung verschiedener Dinge. Einige komprimieren Byte-Arrays, manche komprimieren Strings, manche komprimieren Dateien. Ich würde dieses einzelne Dienstprogramm in mehrere Dienstprogramme brechen:

public static class ByteCompression 
{ 
    public static Byte[] Compress(Byte[] input); 
    public static Byte[] Compress(Byte[] input, Format format); 
    public static Byte[] Compress(Byte[] input, Format format, Level level); 

    public static Byte[] Decompress(Byte[] input); 
    public static Byte[] Decompress(Byte[] input, Format format); 
} 

public static class StringCompression 

    public static String Compress(String input); 
    public static String Compress(String input, Format format); 
    public static String Compress(String input, Format format, Level level); 

    public static String Decompress(String input); 
    public static String Decompress(String input, Format format); 
} 

public static class FileCompression 
{ 
    public static void Compress(String input_file_path, String output_file_path); 
    public static void Compress(String input_file_path, String output_file_path, Format format); 
    public static void Compress(String input_file_path, String output_file_path, Format format, Level level); 

    public static void Decompress(String input_file_path, String output_file_path); 
    public static void Decompress(String input_file_path, String output_file_path, Format format); 
} 

public static FolderCompression 
{ 
    public static void Compress(String input_folder_path, String output_file_path); 
    public static void Compress(String input_folder_path, String output_file_path, Format format); 
    public static void Compress(String input_folder_path, String output_file_path, Format format, Level level); 

    public static void Decompress(String input_file_path, String output_file_path); 
    public static void Decompress(String input_file_path, String output_file_path, Format format); 
} 

Die obigen Utility-Klassen Wiederholung zu reduzieren, besser Zweck einkapseln, sind mehr Zusammenhalt mit ihrem Mitglied Methoden und sind klarer in der Absicht. Sie haben vier statische Utility-Typen anstelle von einem, aber Sie brechen nicht so viele Regeln/Best Practices auf diese Weise. Versuchen Sie, monolithische Dienstklassen zu vermeiden. Wenn Sie können, finden Sie eine Möglichkeit, diese Instanzklassen anstelle von statischen Klassen zu erstellen, insbesondere wenn auf Klassenebene gemeinsame Daten vorhanden sind, die für jede Komprimierungs-/Dekomprimierungsmethode verwendet werden. Das wird die Fadensicherheit verbessern.

EDIT:

Eine ideale Implementierung Erweiterungsmethoden verwenden würde, wie Andy kommentiert. Die Datei- und Ordnerkomprimierung ist etwas schwieriger als Erweiterungen zu implementieren, aber ich habe es ausprobiert. Die folgenden Beispiele erreichen besser, was ich anstrebte: Trennung des Substantivs (oder Subjekts) von Verb (oder Operation), wodurch eine sauberere API bereitgestellt wird, die letztendlich weniger Wiederholungen enthält, die Trennung der Interessen aufrechterhält und ordnungsgemäß gekapselt ist.

public static class ByteCompressionExtensions 
{ 
    public static byte[] Compress(this byte[] input); 
    public static byte[] Compress(this byte[] input, Format format); 
    public static byte[] Compress(this byte[] input, Format format, Level level); 

    public static byte[] Decompress(this byte[] input); 
    public static byte[] Decompress(this byte[] input, Format format); 
} 

// In use: 
byte[] myArray = new byte[] { ... }; 
byte[] compArray = myArray.Compress(); 
// Subject (noun) -----^  ^----- Operation (verb) 


public static class StringCompressionExtensions 
{ 
    public static byte[] Compress(this string input); 
    public static byte[] Compress(this string input, Format format); 
    public static byte[] Compress(this string input, Format format, Level level); 

    // Extension method fail!! :(:(This conflicts with Decompress from the class above! 
    public static string Decompress(this byte[] input); 
    public static string Decompress(this byte[] input, Format format); 
} 

// In use: 
string myStr = "A string!"; 
byte[] compArray = myStr.Compress(); 
// Subject (noun) ---^  ^----- Operation (verb) 
myStr = compArray.Decompress(); // Fail! :(


public static class FileCompressionExtensions 
{ 
    public static void Compress(this FileInfo input, FileInfo output); 
    public static void Compress(this FileInfo input, FileInfo output, Format format); 
    public static void Compress(this FileInfo input, FileInfo output, Format format, Level level); 

    public static void Decompress(this FileInfo input, FileInfo output); 
    public static void Decompress(this FileInfo input, FileInfo output, Format format); 
} 

// In use: 
FileInfo myFile = new FileInfo(input_file_path); 
FileInfo myCompFile = new FileInfo(output_file_path); 
       myFile.Compress(myCompFile); 
// Subject (noun) --^  ^----- Operation (verb) 
       myCompFile.Decompress(myFile); 


public static class FolderCompressionExtensions 
{ 
    public static void Compress(this DirectoryInfo input, DirectoryInfo output); 
    public static void Compress(this DirectoryInfo input, DirectoryInfo output, Format format); 
    public static void Compress(this DirectoryInfo input, DirectoryInfo output, Format format, Level level); 

    public static void Decompress(this DirectoryInfo input, DirectoryInfo output); 
    public static void Decompress(this DirectoryInfo input, DirectoryInfo output, Format format); 
} 

// In use: 
DirectoryInfo myDir = new DirectoryInfo(input_folder_path); 
DirectoryInfo myCompDir = new DirectoryInfo(output_folder_path); 
       myDir.Compress(myCompDir); 
// Subject (noun) --^  ^----- Operation (verb) 
       myCompDir.Decompress(myDir); 

Answer 2

Eine offensichtliche Verbesserung wird mit VS2010 sein, wo Sie optionale Parameter haben können.

Eine andere Sache, die hilfreich sein könnte, ist, Erweiterungs-Methoden anzubieten, so dass ich tun konnte: input_folder_path.CompressFolder (output_file_path) .DecompressFolder (outputfile);

Dies würde mir erlauben, zu komprimieren und dann dekomprimieren, was komprimiert wurde, um die Komprimierung zu überprüfen.

Was ist, wenn ich einen Ordner komprimieren und ihn auf die gleiche Ebene wie den Pfad der Eingabedatei stellen möchte, warum muss ich dann die Ausgabedatei angeben?

Also, wenn ich CompressFolder (@ "C: \ input_folder") und lassen Sie es so, dann würde es C: als Ausgabepfad verwenden.

Answer 3

Ein anderer Verbesserer nicht in .net 4 könnte sein, eine Klasse CompressInfo zu erstellen, die String input_folder_path, String output_file_path, Format format, Level level Eigenschaften darin haben, und nur eine Methode zu haben, die überprüft wird, ist Eigenschaften null oder nicht.

Answer 4

von jrista Im Anschluss an, würde ich sie erben, weil ich es annehmen kann einige gemeinsame Funktionalität ist:

abstract class CompressorBase<T> { } 

mit

Und dann betrachten ein Standardverfahren in der Form:

public CompressionResult Compress (T toCompress, CompressionParams paramaters) 
{ 
} 

Dann, zumindest die Klasse selbst macht klare Entscheidungen darüber, was zu tun ist, einfach basierend auf Änderungen an der "CompressionParams" -Klasse.

Es ist ziemlich nett, weil Sie die öffentliche API nicht mehr ändern müssen, nur Änderungen an dieser Klasse vornehmen und der 'Compressor' wird den Rest herausfinden.

Answer 5

Für die Komprimierungsmethoden sollten Informationen wie Komprimierung zurückgegeben werden. Ähnlich zur Dekomprimierung eine Statusanzeige, wenn irgendeine Datei/Ordner aus irgendeinem Grund nicht dekomprimiert wurde.

Bei der Komprimierung/Dekomprimierung gibt es normalerweise Fälle, in denen die Methode nicht erfolgreich sein sollte. Wenn Sie beispielsweise versuchen, eine verwendete Datei zu komprimieren oder den Speicherort zu dekomprimieren, kann möglicherweise etwas überschrieben werden. Da dies keine "Ausnahmen" sind, sollten Sie in solchen Fällen keine Ausnahmen auslösen und die Informationen entweder als Rückgabewert oder als "out" -Parameter empfehlen.

Answer 6

Zum einen würde ich empfehlen, einen Blick auf diese hervorragende Präsentation von Casey Muratori mit: http://www.mollyrocket.com/873
(Sie haben die Folien und die Audio separat leider folgen)

Wenn Sie halten eine monolithische Klasse planen, meine persönliche Präferenz wäre:

public static Byte[] CompressGzip(Byte[] input); 
public static Byte[] CompressGzip(Byte[] input, Level level); 

public static Byte[] DecompressGzip(Byte[] input); 

public static String CompressGzip(String input); 
public static String CompressGzip(String input, Level level); 

etc 

ie. I wissen, dass sie Bytes sind, der Compiler weiß, dass sie Byte sind, warum muss ich es eingeben? Es ist jedoch wichtig, Gzip vorne und in der Mitte zu halten, da es eine Anforderung ist, dass mit Gzip komprimierte Daten mit demselben dekomprimiert werden. Dies funktioniert natürlich nicht, wenn Sie Byte-Arrays zu Strings oder einer Kombination daraus codieren können.

Ie. sonst sieht dieser Code verdächtig nicht-verdächtig:

Format f = Format.NotDefault; 

// Use our non-standard compression 
String compressed = Compress("my name", f); 

// more code, or transfer across the network 

// Uh oh! Decompression failed. 
// The default parameters are broken in this case! 
String decompressed = Decompress(compressed); 

Durch das Verfahren im Namen setzen, stellen Sie sicher, dass jeder denkt über das, was die komprimierten Bytes formatiert sind in

Ferner lassen Sie Raum zusätzliche hinzuzufügen. Kompressionsoptionen für verschiedene Engines - zB LZMA's Dictionary Size Parameter.

Answer 7

One Refactoring Verbesserung, die ich machen könnte, ist:

public sealed class CompressOptions 
{ 
    public Format Format { get; set; } 
    public Level Level { get; set; } 
} 

Sie dann auf 2 Methoden pro Kompression Ziel zu reduzieren. Am Beispiel der Byte [] Kompressoren.

public static Byte[] Compress(Byte[] input) 
{ 
    Compress(input, new CompressOptions { Format=Zip, Level=Normal }); 
} 
public static Byte[] Compress(Byte[] input, CompressOptions options) 
{ 
    if(options == null) 
     throw new ArgumentNullException("options"); 

    // compress-away 
} 

Code Anrufer kann dann unabhängig von Optionen, die sie wollen nutzen, ohne dass Sie Überschreibungen für jede denkbare Szenario bereitzustellen, die einmal dupliziert werden nur scheinen pro Szenario (Byte, Strings, Dateien).

Byte[] b = GetSomeData(); 
var result = Compress(b, new CompressOptions { Format=Gzip }); 
var result2 = Compress(b, new CompressOptions { Level=Store }); 
var result3 = Compress(b); 

Vielleicht möchten CompressOptions unveränderlich sowie machen (dh einmal gesetzt, kann ein Wert nicht geändert werden)

Diese Konstruktion ermöglicht auch die Compress Optionen in Code übergeben werden, die etwas komprimieren muss , ohne dass Sie wissen müssen, welche Komprimierung verwendet werden soll.

Für andere Kompressoren, die möglicherweise mehr Optionen benötigen, können Sie Unterklassen von CompressOptions (öffnen Sie es jedoch zuerst und versiegeln Sie alle Blattklassen). Es gibt eine Reihe von Variationen hier.

Answer 8

In unserem CORE Bibliothek bieten wir diese Klasse als Abstraktion 20.000 Linie ....

20k Linien? Ernst? Es ist dann nicht die API-Oberfläche, die dein Problem ist. Ich meine sicher, CompressString(string) ruft gerade in CompressString(string, Format, Level) - richtig? Und sicherlich CompressString(string, Format, Level) besteht im Wesentlichen aus:

byte[] b = System.Text.Encoding.Default.GetBytes(input); 
byte[] c = CompressBytes(b, format, level); 
return Convert.ToBase64(c); 

, die alle drei Linien ist - mit den temporären Variablen. Ich kann mir ähnliche Implementierungen vorstellen.

Also - das führt mich zu der Annahme, dass rund 19.500 Zeilen sein muss. Ich würde sagen das ist Ihr Problem.