2015-08-18 1 views
13

У меня есть уродливый код и хотят, чтобы реорганизовать его:Обработка Java исключения, пойманное в конструкторах, с конечными членами

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

Любые советы?

P.S. У меня есть ограничение, которое конструкторы не бросают, иначе все будет легко.

+0

Всегда считал, что вместо того, чтобы использовать конечные переменные экземпляра вы можете использовать неконечное? Если вы хотите убедиться в неизменности вне класса, верните защитную копию в getter. –

+0

Является ли требование 'address == null', если создание сокета не удалось? – dhke

+0

Почему у вас нет другого 'tmp_address' и назначить' address = tmp_address' в самом конце? –

ответ

13

Позвольте конструктору исключить исключения. Оставшийся final не назначен, если конструктор не заканчивается нормально, так как в этом случае объект не возвращается.

Самая уродливая часть вашего кода - это перехват исключений в конструкторе, а затем возврат существующего, но недопустимого экземпляра.

+0

Спасибо, я отдам эту серьезную мысль. – fadedbee

+0

Если я делаю 'новый Layer0 (новый Layer1 (новый Layer2 (params ...), params ...), params ...)' весь стек будет терпеть неудачу, а Layer1 не будет знать, какие параметры класса и конструктора он должен использовать для создания нового Layer2. – fadedbee

+0

Подумав об этом, основной причиной является то, что мне нужно дать каждому слою в стеке протокола способ построить его дочерний элемент, а не дать ему уже построенный ребенок. Это сложно ... – fadedbee

4

Две версии конструктора, где ваши атрибуты остаются окончательными. Обратите внимание, что я не считаю ваш оригинальный подход вообще «уродливым». Его можно улучшить, поскольку блок try-catch идентичен для обоих исключений. Это становится моим конструктором # 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; 
} 
2

Нет оснований для того, чтобы этот конструктор вообще обнаружил исключения. Объект, полный значений null, не имеет никакого значения для приложения. Конструктор должен throw это исключение. Не поймать его.

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 
    } 
  • Наконец, ИМХО, печать трассировки стека в таких методов не является хорошей идеей.
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; 
     } 
    } 
    ... 

 Смежные вопросы

  • Нет связанных вопросов^_^