用于7zip压缩的Python函数

Question

我用Python编写了一个函数来调用7zip来压缩文件夹。代码工作并能够执行任务，但是我不知道这是否是最优的方法。是否有任何方法来改进代码，是否有多余的步骤？

def Compress_7z(Initial_path,Container_Name,File_List, compression_level = -1,DAC = False):
 """Compression level = -1 standard, 0-9 compression level
       DAC : Delete After Compression - Remove File after being compressed
    """ 
    try:
        chdir(Initial_path)
    except:
        return('Path does not exist')

    # write a listfile
    with open('list.txt', 'w') as f:
        for item in File_List:
            f.write("%s\n" % item)

    cmd = ['7z', 'a', Container_Name,'@list.txt']

    if compression_level != -1 and compression_level <= 9:
        cmd.append('-mx{0}'.format(compression_level))
    elif compression_level > 9 or compression_level < -1:
        return("Compression not standard: aborting compression")

    system = subprocess.Popen(cmd, stderr=subprocess.STDOUT, stdout=subprocess.PIPE,shell=True)

    while system.poll() == None:  # .poll() will return a value once it's complete. 
        time.sleep(1)
    if DAC:
        for f in File_List:
            remove(f)

    # Clean up from compression
    remove('list.txt')
    return(system.communicate())

Answer 1

简单地阅读代码已经有一些突出的地方(一般来说，这都符合本文件描述的风格，这是Python代码遵循的标准)：

• Compress_7z应该以小写开头，可能也应该说是compress_7zip。我同意，考虑到格式名称，字面上的"7“在这里是合适的。
• DAC是一个糟糕的变量名，因为您必须读取docstring才能开始了解这个参数正在做什么。一个更好的名字可能是，嗯，delete_files_after。
• 其余的参数也都应该是小写的。
• 尝试应用一致的格式设置，或者使用IDE或命令行工具来为您做这件事(例如，在一些逗号之后，现在没有空格，还有一些引号是单引号，有些是双引号，没有很好的理由)。
• docstring不是很标准，尽管它至少提到了参数的值范围，这显然是很好的。

接下来是实际功能：

当然，使用7z实用程序也是一种选择，不过据我所见，也有一些纯粹的Python库。我指出这一点的原因是，其中一个原因是命令行实用程序可能没有安装，例如pip或类似的Python工具可能会使处理依赖关系变得更容易。

但是，既然您已经确定了它，那么关于当前实现的一些要点如下：

• 库函数中的chdir几乎肯定是个坏主意。完全没有预料到“压缩这些文件”也意味着“更改我当前的工作目录”。相反，请确保该函数不通过可靠地指定文件名(即使用绝对路径名，或者仅指定被调用进程的工作目录)来工作。
• 使用字符串作为错误值是一个很大的否定。使用异常，或者如果您绝对不想这样做，则使用结构正确的返回值。现在，您无法将工具的返回值与字符串"Path does not exist"区分开来，这通常是要避免的。
• 如果压缩级别是字符串，则不会清除list.txt文件。这就是为什么您通常希望正确地处理异常，查找try，catch和finally工作是如何处理异常的。
• 最后，似乎subprocess.Popen调用工作太多了。如果您不需要与被调用的进程通信，只需运行它并等待结果。time.sleep是一种极其浪费的方法来完成同样的任务。相反，我认为您可能只需将subprocess.run与cwd参数一起使用(参见上面)并完成它。您也不需要shell=True，因为调用根本不使用任何shell特性。