14

我有一些丑陋的代码,想重构它:

public class UdpTransport extends AbstractLayer<byte[]> {
    private final DatagramSocket socket;
    private final InetAddress address;
    private final int port;
    /* boolean dead is provided by superclass */

    public UdpTransport(String host, int port) {
        this.port = port;
        InetAddress tmp_address = null;
        try {
            tmp_address = InetAddress.getByName(host);
        } catch (UnknownHostException e) {
            e.printStackTrace();
            dead = true;
            socket = null;
            address = null;
            return;
        }
        address = tmp_address;
        DatagramSocket tmp_socket = null;
        try {
            tmp_socket = new DatagramSocket();
        } catch (SocketException e) {
            e.printStackTrace();
            dead = true;
            socket = null;
            return;
        }
        socket = tmp_socket;
    }
    ...

导致丑陋的问题是final成员之间的交互和捕获的异常。如果可能的话,我想保留成员final

我想按如下方式编写代码,但 Java 编译器无法分析控制流 -address无法再次分配,因为必须抛出第一次尝试分配才能使控制到达catch子句。

public UdpTransport(String host, int port) {
    this.port = port;
    try {
        address = InetAddress.getByName(host);
    } catch (UnknownHostException e) {
        e.printStackTrace();
        dead = true;
        address = null; // can only have reached here if exception was thrown 
        socket = null;
        return;
    }
    ...

Error:(27, 13) error: variable address might already have been assigned

有什么建议吗?

PS我有一个约束,即构造函数不会抛出 - 否则这一切都很容易。

4

5 回答 5

14

让构造函数抛出异常。final如果构造函数没有正常终止,则保持未赋值是可以的,因为在这种情况下没有返回任何对象。

代码中最难看的部分是在构造函数中捕获异常,然后返回现有但无效的实例。

于 2015-08-18T09:59:15.410 回答
8

如果您可以自由使用私有构造函数,则可以将构造函数隐藏在公共静态工厂方法后面,该方法可以返回UdpTransport类的不同实例。让我们说:

public final class UdpTransport
        extends AbstractLayer<byte[]> {

    private final DatagramSocket socket;
    private final InetAddress address;
    private final int port;
    /* boolean dead is provided by superclass */

    private UdpTransport(final boolean dead, final DatagramSocket socket, final InetAddress address, final int port) {
        super(dead);
        this.socket = socket;
        this.address = address;
        this.port = port;
    }

    public static UdpTransport createUdpTransport(final String host, final int port) {
        try {
            return new UdpTransport(false, new DatagramSocket(), getByName(host), port);
        } catch ( final SocketException | UnknownHostException ex ) {
            ex.printStackTrace();
            return new UdpTransport(true, null, null, port);
        }
    }

}

上面的解决方案可能有以下注意事项:

  • 它只有一个构造函数,仅将参数分配给字段。因此,您可以轻松拥有final字段。这与我记得在 C# 和可能的 Scala 中称为主构造函数的内容非常相似。
  • 静态工厂方法隐藏了UdpTransport实例化的复杂性。
  • 为简单起见,静态工厂方法返回一个实例,将两者都socket设置address为真实实例,或者将它们设置为null指示无效状态。因此,此实例状态可能与您问题中的代码产生的状态略有不同。
  • 这样的工厂方法允许您隐藏它们返回的实例的真实实现,因此以下声明是完全有效public static AbstractLayer<byte[]> createUdpTransport(final String host, final int port)的:(注意返回类型)。这种方法的强大之处在于,如果可能的话,您可以根据需要将返回值替换为任何子类,除非您使用- 特定的UdpTransport公共接口。
  • 另外,如果您对无效状态对象感到满意,我猜,那么无效状态实例不应该包含允许您进行以下操作的真实端口值(假设-1可以指示无效端口值,或者Integer如果您可以自由,甚至可以为空更改类的字段,原始包装器对您没有限制):
private static final AbstractLayer<byte[]> deadUdpTransport = new UdpTransport(true, null, null, -1);
...
public static AbstractLayer<byte[]> createUdpTransport(final String host, final int port) {
    try {
        return new UdpTransport(false, new DatagramSocket(), getByName(host), port);
    } catch ( final SocketException | UnknownHostException ex ) {
        ex.printStackTrace();
        return deadUdpTransport; // it's safe unless UdpTransport is immutable
    }
  • 最后,恕我直言,以这种方法打印堆栈跟踪不是一个好主意。
于 2015-08-18T10:43:28.183 回答
4

构造函数的两个版本,您的属性保持最终状态。请注意,我不认为您的原始方法通常“丑陋”。不过它可以改进,因为两个异常的 try-catch-block 是相同的。这成为我的构造函数#1。

public UdpTransport(String host, int port) {
    InetAddress byName;
    DatagramSocket datagramSocket;

    try {
        byName = InetAddress.getByName(host);
        datagramSocket = new DatagramSocket(); 
    } catch (UnknownHostException | SocketException e) {
        e.printStackTrace();
        dead = true;
        datagramSocket = null;
        byName = null;
    }
    this.port = port;
    this.socket = datagramSocket;
    this.address = byName;
}

public UdpTransport(int port, String host) {

    this.port = port;
    this.socket = createSocket();
    this.address = findAddress(host);
}

private DatagramSocket createSocket() {
    DatagramSocket result;
    try {
        result = new DatagramSocket(); 
    } catch (SocketException e) {
        e.printStackTrace();
        this.dead = true;
        result = null;
    }
    return result;
}

private InetAddress findAddress(String host) {
    InetAddress result;
    try {
        result = InetAddress.getByName(host);
    } catch (UnknownHostException e) {
        e.printStackTrace();
        this.dead = true;
        result = null;
    }
    return result;
}
于 2015-08-18T09:56:26.613 回答
2

此构造函数根本没有任何正当理由来捕获异常。一个充满null值的对象对应用程序没有任何用处。构造函数应该抛出该异常。抓不到。

于 2015-08-18T10:09:59.507 回答
0

像这样的东西怎么样:

public class UdpTransport extends AbstractLayer<byte[]> {
    private final DatagramSocket socket;
    private final InetAddress address;
    private final int port;

    public static UdpTransport create(String host, int port) {
        InetAddress address = null;
        DatagramSocket socket = null;
        try {
            address = InetAddress.getByName(host);
            socket = new DatagramSocket();
        } catch (UnknownHostException | SocketException e) {
            e.printStackTrace();
        }
        return new UdpTransport(socket, address, port);
    }

    private UdpTransport(DatagramSocket socket, InetAddress address, int port) {
        this.port = port;
        this.address = address;
        this.socket = socket;
        if(address == null || socket == null) {
           dead = true;
        }
    }
    ...
于 2015-08-18T10:47:28.130 回答