Этот способ исключений обрабатывает хорошую практику?

Question

Вы бы что-нибудь изменили в этом коде?

class LocalPort {
public:
    LocalPort(int portNumber) {
      innerPort = new ACMEPort(portNumber);
   }

    void Open() {
      try {
          innerPort->Open();
      } 
      catch (DeviceResponseException& e) {
          throw PortDeviceFailure(e);
      } 
      catch (ATM1212UnlockedException& e) {
         throw PortDeviceFailure(e);
      } 
      catch (GMXError& e) {
         throw PortDeviceFailure(e);
      }
   } 
private: 
    ACMEPort* innerPort;
};

/////////

try {
    LocalPort* port = new LocalPort(12);
    port->Open();
} 
catch (bad_alloc& e) {
    ReportError(e);
    logger.Log("Wyjątek alokacji pamięci", e);
    delete port;
}
catch (PortDeviceFailure& e) {
    ReportError(e);
    logger.Log(e.getMessage(), e);
    delete port;
} 

Я пытался выше сделать так, чтобы код ниже выглядел и действовал лучше.

try {
    ACMEPort* port = new ACMEPort(12);
    port->Open();
} 
catch (bad_alloc& e) {
    ReportPortError(e);
    logger.Log("Wyjątek alokacji pamięci", e);
    delete port;
}
catch (DeviceReponseException& e) {
    ReportPortError(e);
    logger.Log("Wyjątek odpowiedzi urządzenia", e);
    delete port;
} 
catch (ATM1212UnlockedException& e) {
    ReportPortError(e);
    logger.Log("Wyjątek odblokowania", e);
    delete port;
} 
catch (GMXError& e) {
    ReportPortError(e);
    logger.Log("Wyjątek odpowiedzi urządzenia");
    delete port;
}
catch (...) {
    ReportPortError(0);
    logger.Log("Wyjątek nieznanego pochodzenia");
    delete port;
}

Удалось ли мне? Первый лучше второго? Что ты думаешь?

Answer 1

Похоже, у вас плохо разработаны классы исключений.Почему бы вам просто не добавить функцию-член getMessage в вашу иерархию исключений (предпочтительно в базовом классе все ваши исключения происходят из - std::exception предоставляет метод what, который возвращает сообщение об ошибке).Таким образом, вы можете просто перехватить все исключения в одном операторе и обработать их одинаково.

class DeviceResponseException : public std::runtime_error {
public:
    DeviceResponseException() : std::runtime_error("Some error message") {}
};

class ATM1212UnlockedException : public std::runtime_error {
public:
    ATM1212UnlockedException() : std::runtime_error("Some other error message") {}
};

Затем в своем коде вы можете сделать это:

try {
    std::auto_ptr<ACMEPort> port(new ACMEPort(12));
    port->Open();
}
catch( std::runtime_error & e) {
    ReportPortError(e);
    logger.Log(e.what(), e);
}

Поскольку все ваши классы исключений являются производными от std::runtime_error, эта catch -класс перехватывает их все.Это нормально, так как обработка одинакова для всех случаев.Если у вас есть конкретные исключения, требующие специальной обработки, вы добавляете дополнительные catch -классы.

Кроме того, вместо вызова delete port в обработчике исключений, вы должны использовать std::auto_ptr или boost::scoped_ptr иличто-то в этом роде.Читайте о RAII .

Answer 2

Да, я бы:

• имел один единственный класс исключений и, таким образом:
• не переводил исключения из одного типа в другой
• ловил исключения намного вышев коде и, таким образом:
• не заканчивает тем, что пишет больше кода обработки исключений, чем реальный код
• , и я не буду создавать объекты с использованием new, и если бы я это сделал, я использовал бы умные указатели

В принципе, я думаю, вы совершенно не поняли, как следует использовать исключения.

Лучшее единственное изменение, которое вы можете сделать в своем коде, - это заменить:

ACMEPort* port = new ACMEPort(12);
port->Open();

на:

ACMEPort port(12);
port.Open();

и аналогично в других местах - тогда необходимость отлавливать исключенияуходит.

Answer 3

Я бы переместил обработчики исключений в класс и позволил бы классу обрабатывать ошибки вместо того, чтобы заставлять всех вызывающих его обрабатывать. Таким образом, вы можете очистить указатель (если вам действительно нужен указатель по какой-то причине), проверив значение результата Open ():

class LocalPort {
public:
    LocalPort(int portNumber) {
      innerPort = new ACMEPort(portNumber);
   }

    bool Open() {
      try {
          innerPort->Open();
          return true;
      } 
      catch (DeviceResponseException& e) {
          throw PortDeviceFailure(e);
      } 
      catch (ATM1212UnlockedException& e) {
         throw PortDeviceFailure(e);
      } 
      catch (GMXError& e) {
         throw PortDeviceFailure(e);
      }
      catch (bad_alloc& e) {
         ReportError(e);
         logger.Log("Wyjątek alokacji pamięci", e);
      }
      catch (PortDeviceFailure& e) {
         ReportError(e);
         logger.Log(e.getMessage(), e);
      }
      return false;
   } 
private: 
    ACMEPort* innerPort;
};

LocalPort* port = new LocalPort(12);
if (!port->Open()) {
  delete port;
}