这个API能否改善？

Question

在我们的CORE库中，我们提供了这个类作为一个2万行的抽象。你能看出设计上有什么问题吗？

注意1：这个类有一个SharpZipLib支持。

注意2：SharpZipLib大约有2万行代码。

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);
}

- ChaosPandion

---

我认为你应该去掉“主观”标签。这是一个好问题，我认为与主观性问题相关的不愉快的污名化是不必要的。此外，这并不真正是主观的问题 - 这是一个关于重构的问题，而这些都是严肃的问题。无论如何，+1。 - Rob

Answer 1

我建议将这个单一的类拆分成几个类。总的来说，静态实用程序类会违反许多规则，其中最重要的是关注点分离。虽然是的，这个类中的所有方法都涉及压缩，但它们所关注的是压缩不同的内容。有些方法压缩字节数组，有些压缩字符串，有些压缩文件。我会将这个单一的实用程序拆分成多个实用程序：

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);
}

以上的实用类减少了重复代码，更好地封装了目的，与其成员方法更加协调，在意图上更加清晰。你有四个静态实用程序类型而不是一个，但这样做没有违反太多规则/最佳实践。尽量避免单块式的、全功能的实用程序类。如果可以的话，找到一种方法将它们变为实例类，而不是静态类，特别是如果在整个压缩/解压缩方法中使用了任何共享数据，则会提高线程安全性。

编辑：

如andy所评论的那样，更理想的实现方式应该使用扩展方法。文件和文件夹压缩作为扩展实现起来有点困难，但我已经尝试过了。以下示例更好地实现了我想要的：将名词（或主题）与动词（或操作）分离，提供了一个更清洁的API，最终具有更少的重复性，保持关注点的分离，并且被正确地封装。

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);

- jrista

---

如果你打算按类型进行分割，我建议完全放弃使用API，而是通过扩展方法来实现所有功能，例如SomeString.Compress()、SomeBytes.Compress()等。 - andy

我会将这个答案与上面的答案结合起来。但一定要先拆分相关功能 - 一个文件里有20,000行代码太多了。 - Jamie Penney

如果您使用正确版本的C#，扩展方法是一种选择。然而，考虑到文件和文件夹压缩的性质，这将破坏本来应该是一个清晰API的东西，因为没有合适、合理的方式将它们实现为扩展方法。 - jrista

你可以采取的另一个步骤是在名称中添加压缩类型（如果它是众所周知的）。例如：CompressZip和DecompressZip。 - Chris Dunaway

@ChaosPandion：我觉得你还是误解了我想表达的意思。你把这个问题归类为“压缩”，这是一个非常广泛的问题，你可以将很多功能都归为一个静态类下面。试着从更细粒度的角度来看待它：字节数组压缩、字符串压缩、文件和文件夹压缩。很多架构都归结于如何对事物进行分类。你的分类不能过于宽泛，但正如你所指出的，也不能过于细致。找到合适的平衡，你将会获得好处。 - jrista

Answer 2

在VS2010中，您可以使用可选参数来进行明显的改进。

另一个有用的功能是提供扩展方法，这样我就可以这样做：input_folder_path.CompressFolder(output_file_path).DecompressFolder(outputfile);

这使我能够压缩，然后解压已压缩的内容以验证压缩。

如果我想压缩一个文件夹并将其放在与输入文件路径相同的级别上，为什么我需要指定输出文件呢？

因此，如果我执行CompressFolder(@"C:\input_folder")并保留它，那么它将使用C：作为输出路径。

- James Black

---

我喜欢API返回路径的想法。当您压缩文件或文件夹时。 - ChaosPandion

请解释为什么可选参数会是“显而易见的改进”。我不认为无序参数真正改善了这个API。此外，自动将c：\ input_folder压缩到c：\ input_folder.zip，如果有的话，只是另一种覆盖方法。 - Robert Paulson

如果我调用CompressString("some input string")，那么格式和级别将具有默认值，因为我希望所有内容都导致具有3个参数的Compress方法。如果我有可选参数，那么我可以将这些默认值放在参数列表中，并将函数数量减少到一个，但仍然具有重载的优点，即程序员只需要传递一个参数。假设输出目录只是简化了原始请求，以便了解如何使API更好。 - James Black

你可以在不使用可选参数的情况下立即实现。请查看我的答案。 - Robert Paulson

Answer 3

首先，我建议您查看Casey Muratori的这个优秀演示：http://www.mollyrocket.com/873（不幸的是，您必须分别跟随幻灯片和音频）。

如果您打算保留一个单olithic类，我个人的偏好是：

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

例如，我知道它们是字节，编译器也知道它们是字节，为什么我还要输入它们呢？然而，将Gzip置于中心位置非常重要，因为使用Gzip压缩的数据需要使用相同的方法进行解压缩。当然，如果您可以将字节数组编码为字符串或任何组合，则此方法无效。

例如，否则此代码看起来可疑但实际上并不可疑：

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);

通过在名称中添加方法，您可以确保每个人都考虑压缩字节的格式。此外，您留出了为不同引擎添加额外压缩选项的空间-例如LZMA的字典大小参数。

- kibibu

Answer 4

我可能会进行一项重构改进：

一个重构改进我可能会做的是：

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

你可以将每个压缩目标减少到2种方法。以Byte[]压缩器为例。

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
}

调用者代码可以使用任何选项，而无需您提供每种情况的覆盖，这似乎只是针对每种情况（字节，字符串，文件）重复一次。

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

你可能想使CompressOptions也是不可变的（即一旦设置，值就无法更改）。

这种设计还允许将压缩选项传递到需要压缩某些内容的代码中，而不需要知道要使用什么压缩方式。

对于可能需要更多选项的其他压缩器，你可以从CompressOptions派生子类（但首先要取消其密封，并封闭任何叶类）。这里有许多变化。

- Robert Paulson

---

我也建议与@jrista的答案结合使用。 - Robert Paulson

我很喜欢你的答案，特别是因为你提供了一个选项，使得代码的某些部分不需要详细信息，这样它可以使用 DI 进行注入，以便用户可以设置这些参数。 - James Black

Answer 5

在 .net 4 中的另一个不良做法是创建一个名为 CompressInfo 的类，其中包含 String input_folder_path, String output_file_path, Format format, Level level 属性，并只有一个方法来检查这些属性是否为空。

- Arsen Mkrtchyan

---

我宁愿选择重载而不是一个单一的庞大方法，该方法必须每天检查空值。重载简洁明了，不需要事先创建某些类型以传递到调用中。 - jrista

为什么？拥有最多参数的方法仍然必须查找其他参数中的null吗？ - sirrocco

是的，但他只会在API中向终端用户提供2种方法。 - Arsen Mkrtchyan

只是为了记录，我是在回复 jrista :) - sirrocco

我同意jrista的观点。我会继续使用重载...当然，直到C# 4发布为止。 - ChaosPandion

Answer 6

继续jrista的话题，我会继承它们，因为我可以假设存在一些共同的功能：

abstract class CompressorBase<T> { }

然后考虑采用形式为：

的标准方法。

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

那么，至少类本身正在基于'CompressionParams'类的更改做出明确的决策。

这很好，因为您不再需要在公共API中进行更改，只需对该类进行更改，'Compressor'将解决其余问题。

- Noon Silk

---

这使得CompressionParams成为API的一部分，用户必须适当地配置它以适应不同类型的T...功能较少，但在我看来这暴露了太多内容给外部。 - MaxVT

是的，但调用者已经传递了很多参数，这将极大地简化您的API。因此，您的实例化类似于ByteCompressor<byte[]> b = new ByteCompressor<byte[]>(); b.Compress(myBytes, params);。泛型略显冗余，但这只是由于C#中缺乏对该继承风格的支持。我个人认为它非常优美 :) - Noon Silk

遵循面向对象的范式会使这变得过于复杂。我们的目标是尽可能简单化。 - ChaosPandion

我并不认为这会使事情过于复杂，但也许这取决于个人喜好。难道您不觉得应该对CompressionParams进行子类化，以便每个子类都有适当的变量吗？以这种方式将所有内容封装成面向对象的形式似乎非常美妙。当然，无论如何，这取决于您的选择 :) 我已经接近编写所有类了，只是想看看它的样子。 - Noon Silk

Answer 7

在压缩方法中，考虑返回压缩实现的信息。同样，在解压缩时，如果由于某些原因未能解压缩任何文件/文件夹，则应指示状态。

在压缩/解压缩的情况下，通常存在无法成功执行该方法的有效情况。例如，尝试压缩正在使用的文件或解压缩位置可能会覆盖某些内容。因为这些不是“异常”，所以在这种情况下不应抛出异常，并建议将信息作为返回值或“out”参数返回。

- Sesh

---

我不同意。这些是调用者应该处理的异常。 - ChaosPandion

试图压缩正在使用的文件正是需要使用异常处理的特殊情况。 - kibibu

Answer 8

在我们的CORE库中，我们提供了一个包含20,000行抽象化代码的类......

20k行？认真吗？那么你的问题就不在API表面上了。我的意思是，肯定是 CompressString(string) 只是在调用 CompressString(string, Format, Level) - 对吧？而且 CompressString(string, Format, Level) 基本上就包括：

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

这只有3行代码，还有临时变量。我可以想到类似的实现方式。

所以，这让我相信CompressBytes(byte[], Format, Level)大约有19,500行代码。我认为那就是你的问题所在。

- Mark Brackett

---

这怎么回答我的问题呢？另外，我更新了我的问题，提到我们使用的是SharpZipLib支持库，实际上大约有20k行代码。 - ChaosPandion