Java应用程序从画廊下载Imgur壁纸

Question

我对现实世界的java相当陌生，所以我尝试重写一个我用普通Java编写的旧应用程序。我做了更多的OO，使用了依赖/构建管理器(Gradle)。现在我只想看看更有经验的开发人员对这个项目有什么看法。

这是完成大部分工作的主要代码片段。欢迎在此发表任何意见。

src/pw/jor/imgurwallpaper/gui/Worker.java : 37

// get url
selection= getGUISelection(selectedURLIndex++);
url = createURL(selection);

// get parser contents
body = Downloader.getPageContents(url);

// parse body for image hashes
ParserAbstract parser = getParser(selection);
parser.parse(body);

// write hashes to file
Writer.writeFiles(parser.getImageHashes());

整项工程

Answer 1

你在这里很有程序。Java确实在某种程度上允许这样做，但它感觉相当笨重。请注意，这里的上下文非常缺乏，所以在查看此代码…时，我将不得不做一些受过良好教育的猜测。

选择= getGUISelection(selectedURLIndex++)；

这有几个问题:你要学的第一件事是“说，不要问”。

无论这段代码在哪里，您都应该通过调用已经完成的方法，将GUISelection传递给ActionListener或类似的代码。selection应该是一个参数。

实际上是…擦掉它：

url = createURL(selection);

这应该是我们现在的方法的第一步。

你需要明确你的责任:一种方法做一件事。它做的很好，…

在对象上工作，如果可以的话，避免静态上下文：

body = Downloader.getPageContents(url)；

这应该使用一个实例：

body = downloader.getPageContents(url);

我是否已经提到过，您应该尽可能地声明变量的用法？

这看起来就像你成了古老惯例的牺牲品，在你的过程的顶端声明你的变量。这使得代码在较长的过程中过于复杂，而且非常混乱。

String body = downloader.getPageContents(url);

会更好。

ParserAbstract解析器=getParser(选择)；

再说一遍:告诉你，别问。另外:处理对象。为选择创建一个特定的分析器是一个单独的责任。虽然我很讨厌他们，但这是工厂的用例：

ParserAbstract parser = ParserFactory.getParser(selection);

除此之外，ParserAbstract听起来像一个abstract class。我敢说作为一个interface可能更好，如果不是这样的话:至少把抽象放在分析器前面。但是，我认为以下是更惯用的java：

Parser parser = ParserFactory.getOrCreateParser(selection);

parser.parse(body)；//将散列写入文件Writer.writeFiles(parser.getImageHashes())；

在我看来这是个反模式。解析器通常不是有状态的，低。它不应该将解析结果保存在某个地方，而应该只返回它们。

再说一遍:告诉你，不要问，你再一次处理一个类，而不是一个与作者一起的实例。

对于java来说，更多的习语是这样的：

writer.writeFiles(parser.parse(body));

或者再伸出来一点，直截了当：

ParseResults imageHashes = parser.parse(body);
writer.writeFiles(imageHashes);