下载功能

Question

我编写了一个下载函数，我想知道我的代码是否干净、可读性和可维护性，或者是否可以使事情变得更简单。请告诉我这个函数是否可以在现实世界中使用。

public static void Download(Uri _downloadUri, string _path, string _passedSong, Song _currentObj) {

        if (_downloadUri != null) {

            if (!string.IsNullOrEmpty(_path) && !string.IsNullOrEmpty(_passedSong)) {

                try {

                    var downloadedData = new byte[0];

                    var webReq = WebRequest.Create(_downloadUri);

                    using (var webResponse = webReq.GetResponse()) {

                        using (var dataStream = webResponse.GetResponseStream()) {

                            //Download chuncks
                            byte[] dataBuffer = new byte[1024];

                            //Get size
                            int dataLength = (int) webResponse.ContentLength;

                            ByteArgs byteArgs = new ByteArgs();

                            byteArgs.downloaded = 0;
                            byteArgs.total = dataLength;

                            if (_bytesDownloaded != null) {
                                _bytesDownloaded(byteArgs, _currentObj);
                            }

                            //Download
                            using (var memoryStream = new MemoryStream()) {

                                while (true) {

                                    int bytesFromStream = dataStream.Read(dataBuffer, 0, dataBuffer.Length);

                                    if (bytesFromStream == 0) {

                                        byteArgs.downloaded = dataLength;
                                        byteArgs.total = dataLength;

                                        if (_bytesDownloaded != null) {
                                            _bytesDownloaded(byteArgs, _currentObj);
                                        }

                                        //Download complete
                                        break;
                                    }
                                    else {

                                        //Write the downloaded data
                                        memoryStream.Write(dataBuffer, 0, bytesFromStream);

                                        byteArgs.downloaded = bytesFromStream;
                                        byteArgs.total = dataLength;

// for download progressbar
                                        if (_bytesDownloaded != null) {
                                            _bytesDownloaded(byteArgs, _currentObj);
                                        }
                                    }
                                }

                                //Convert to byte array
                                downloadedData = memoryStream.ToArray();

                                //Write bytes to the specified file
                                if (Directory.Exists(Settings.Default.downloadDirectory)) {
                                    using (var newFile = new FileStream(_path, FileMode.Create)) {
                                        newFile.Write(downloadedData, 0, downloadedData.Length);
                                    }
                                }
                            }
                        }
                    }
                }
                catch(Exception ex) {
                    ExceptionLog.LogException(ex, "Download()", "in GeneralSettings.cs");
                }
            }
        }
    }

Answer 1

这并不是一个全面的答案，但这是我立即突出的东西。

公共静态无效下载(Uri _downloadUri，string _path，string _passedSong，Song _currentObj)

• 不要在参数名前加上下划线。
• _currentObj不是Song对象的好名称。为什么不把它命名为song呢？
• 还不清楚_passedSong的意义是什么。您可以检查它是空还是空，但是在方法中永远不会使用它。而且--假设它是歌曲的名称--我希望它是Song对象的一个属性，在这种情况下，您不需要单独传递它。

var downloadedData =新字节0；//第9行.downloadedData = memoryStream.ToArray()；//第67行

为什么要在实际使用downloadedData之前声明它呢？将变量定位于它们的使用。(请注意，如果您的方法比较小，这不会是一个问题-这是应该的。)

另一个问题是，您正在创建一个新的字节数组并将其分配给downloadedData，但随后您立即用downloadedData = memoryStream.ToArray()覆盖它，所以创建一个新的字节数组是没有意义的。只需这样做：var downloadedData = memoryStream.ToArray();代替。

//下载chuncks byte[] dataBuffer =新字节1024；

如果“下载”是一个动词，那么注释是误导的；如果它是一个形容词，那么您应该将dataBuffer重命名为描述它的意图/目的的东西，因为您认为需要有一个注释来澄清它。

byteArgs.downloaded = 0；byteArgs.total = dataLength；if (_bytesDownloaded != null) { _bytesDownloaded(byteArgs，_currentObj)；}

有三个地方存在上面的代码，每个地方都有细微的变化。我的问题是，除非byteArgs，否则不会使用_bytesDownloaded != null，但是为byteArgs赋值的工作是在该检查之外完成的。当然，在现实生活中，它并不重要，因为它所做的是微不足道的，但作为一个普遍的原则，它不适合我。

此外，我可能会将这段代码提取到一个方法中，并在三个地方调用该方法，将值作为args传递。事实上，这会比你目前正在做的事情效率更低，尽管在这种特定的情况下它也是可以忽略不计的。

Answer 2

这不是明确的答案，但阅读您的代码，我可以建议如下：

if (Directory.Exists(Settings.Default.downloadDirectory))

这意味着我们在下载后并在保存之前测试目录是否存在，如果目录似乎不存在，那将是浪费时间。

downloadedData = memoryStream.ToArray();

可能类似于memoryStream.CopyTo(newFile)将删除downloadedData字节数组。

byteArgs.total = dataLength;

它出现3次，并且在设置初始值后，dataLength不会改变。

catch(Exception ex) {

由于大多数情况下，捕获一般异常而不是特定异常意味着错误模式，我建议只捕获特定异常，可能是IOException。尽管您正在记录异常，但可能会在日志记录之后重新抛出它们。

希望这能有所帮助。