首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >异常处理,等-如何使这个网页下载程序不“差”?

异常处理,等-如何使这个网页下载程序不“差”?
EN

Code Review用户
提问于 2012-09-07 20:01:25
回答 1查看 551关注 0票数 11

我已经很多年没有做Java编码了,但我想我可以尝试一下工作面试。我有几个问题:

  1. 为什么这种错误处理被认为很糟糕?我该怎么做?
  2. 我应该在finally子句中执行什么错误捕获?
  3. 我怎么能掩盖例外呢?
代码语言:javascript
复制
package jGet;

/**
 * Problem
 * 
 * Create a very simple Java class that will retrieve the resource of any URL (using the HTTP protocol)
 * and save the contents as seen by the browser, to a file.
 * 
 * Restrictions
 * 
 * You are free to use any library/technique, except for java.net.Url, java.net.URI or java.net.UrlConnection.
 * Solutions using these classes will not be accepted.
 * You are free to change the class signature for better error handling and readability.
 * 
 * Initial Class outline
 * 
 * public JGet extends Object {
 *     public JGet( String urlToPage, String saveToFilename ) {
 *     }
 *     public Object getContents() {
 *     }
 * }
 * 
 * Sample Test cases
 * 
 * The class should be able to download the following sample URL's to a file:
 *     http://www.bing.com/
 *     http://www.aw20.co.uk/images/logo.png
 * 
 * Time Allowance
 * 
 * This took me a long time to complete (longer than 30 minutes).
 * I returned the completed .java file to the person who invited me to take this, and
 * this is the feedback I got back:
 *     1) Poor exception handling
 *     2) No finally clause in case of errors
 *     3) getContents() returns a void
 *     4) You should never throw a NullPointerException
 *     5) Exception masking
 *     6) Constructor calls the function getContents() no matter what
 *     7) Poor layout
 *
 */

import java.io.FileOutputStream;
import java.io.IOException;

import org.apache.commons.io.IOUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;

public class JGet {
    private final String urlToPage;
    private final String saveToFileName;

    /**
     * @param urlToPage
     * @param saveToFilename
     */
    public JGet(String urlToPage, String saveToFilename) {
        if (urlToPage == null) {
            throw new IllegalArgumentException("The URL must not be null");
        }
        if (saveToFilename == null) {
            throw new IllegalArgumentException(
                    "The name of the destination file must not be null");
        }
        if (urlToPage.length() == 0) {
            throw new IllegalArgumentException("The URL must not be blank");
        }
        if (saveToFilename.length() == 0) {
            throw new IllegalArgumentException(
                    "The name of the destination file must not be blank");
        }
        this.saveToFileName = saveToFilename;
        this.urlToPage = urlToPage;
        try {
            getContents();
        } catch (IOException e) {
            throw new IllegalArgumentException("The ability to save to the destination file must not be blocked");
        } catch (IllegalArgumentException e) {
            throw new IllegalArgumentException("The URL must not be malformed");
        }
    }

    /**
     * @throws IOException
     * @throws IllegalArgumentException
     */
    public void getContents() throws IOException, IllegalArgumentException {
        HttpClient httpClient = new DefaultHttpClient();
        HttpGet httpGet = new HttpGet(this.urlToPage);
        HttpResponse response = httpClient.execute(httpGet);
        HttpEntity entity = response.getEntity();
        IOUtils.copy(entity.getContent(), new FileOutputStream(this.saveToFileName));
    }

    /**
     * @param args
     */
    public static void main(String[] args) {
        new JGet(null, "bing.html");
        new JGet("http://www.bing.com/", null);
        new JGet("", "bing.html");
        new JGet("http://www.bing.com/", "");
        new JGet("http://www.bing.com/", "readonly.html");
        new JGet("Malformed URL", "bing.html");
        new JGet("http://www.bing.com/", "bing.html");
        new JGet("http://www.aw20.co.uk/a/img/logo.png", "logo.png");
    }

}
EN

回答 1

Code Review用户

发布于 2012-09-07 22:33:47

有几件事我看到如果我在做这件事的话,我会想要重构

  1. getContents()方法抛出异常,在JGet构造函数中以修改错误消息的方式简单地捕获和重新抛出异常,但不包括原因(原始异常)。这会使代码的故障排除变得更加困难,因为您正在模糊有关问题来源的信息。当抛出新异常时,可以将导致异常的异常作为额外的参数传递。
  2. getContents()抓捕并重新抛出例外(S)是否真的是必要的,这是值得怀疑的。如果修改后的错误消息帮助程序员了解为什么它是非法参数的上下文,这可能会有所帮助,但可能不会。在本例中,您捕获一个IOException并以IllegalArugumentException的形式重新抛出它,同时假设唯一可能导致IOException的事情首先是目标文件不可用。当我说你在掩盖根本原因时,这就是我说的那种事情。捕获IllegalArgumentException,然后立即重新抛出它,"The URL must not be malformed"消息也可能会掩盖某些内容,在本例中,由于您没有更改异常类型或添加任何有用的信息,因此看起来完全多余。就让异常像自然的那样滚上链条吧。
  3. 由于可读性的原因,JGet构造函数的前半部分(大约)是应该包装在它自己的方法中的功能(您以后可能会更改定义“非法”参数的内容,但是您希望将该定义保存在确定是否非法的方法中)。

所以,我会把它重构成这样:

代码语言:javascript
复制
/** Does what a JGet does (this is a joke comment)
  * 
  * @param uriToPage The URL to the page you want
  * @param saveToFilename The full or relative path to the file you want to save to
  *
  * @throws IllegalArgumentException if the either the urlToPage or saveToFilename
  *         parameters are null or blank.
  * @throws IOException if an I/O problem occurs either retrieving the results from the URL or saving the results to the file.
  */
public JGet(String urlToPage, String saveToFilename) throws IllegalAgumentException, IOException {
  this.saveToFileName = saveToFilename;
  this.urlToPage = urlToPage;

  validate();
  getContents();
}

/** Some useful documentation...
  */
private void validate() throws IllegalArgumentException {
  // your code for checking for invalid arguments
}


...the rest of it...

我认为这些限制也是非常荒谬的,但是你可以重复使用一个操作系统工具,假设它是可用的,并向curl -L [URL]或类似的东西发出呼叫。

票数 10
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/15420

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档