Good or bad: Calling destructor in constructor [on hold]

  • A+

Break: I don't think it is the same question actually, the other question is a general question about calling destructors manually. This is at the creating process, inside the class itself. Still want to know what happen when you do this, like stated in the question below.

At first, I think it is bad, real bad. Just analysing this piece of code of a constructor (see below), made by two guys and need it to translate it to Delphi object Pascal. It must behave the same like the C-version. I don't like the style, very ugly but never mind.

Another thing, at two stages in the code it calls the destructor when fail (I suppose to close the connection however the destructor is automaticly called when deleted, why want you do this anyway?). I think that is not the way to do it or do miss something inhere?

Also, after calling the destructor, they want to throw an exception (huh?) however I think this will never be executed and cause another exeption when you manually want to access it or want to delete it.

Serial::Serial(   std::string &commPortName,   int bitRate,   bool testOnStartup,   bool cycleDtrOnStartup ) {   std::wstring com_name_ws = s2ws(commPortName);    commHandle =     CreateFileW(       com_name_ws.c_str(),       GENERIC_READ | GENERIC_WRITE,       0,       NULL,       OPEN_EXISTING,       0,       NULL     );    if(commHandle == INVALID_HANDLE_VALUE)     throw("ERROR: Could not open com port");   else {     // set timeouts     COMMTIMEOUTS timeouts;      /* Blocking:         timeouts.ReadIntervalTimeout = MAXDWORD;         timeouts.ReadTotalTimeoutConstant = 0;         timeouts.ReadTotalTimeoutMultiplier = 0;        Non-blocking:         timeouts = { MAXDWORD, 0, 0, 0, 0}; */      // Non-blocking with short timeouts     timeouts.ReadIntervalTimeout = 1;     timeouts.ReadTotalTimeoutMultiplier = 1;     timeouts.ReadTotalTimeoutConstant = 1;     timeouts.WriteTotalTimeoutMultiplier = 1;     timeouts.WriteTotalTimeoutConstant = 1;      DCB dcb;     if(!SetCommTimeouts(commHandle, &timeouts)) {       Serial::~Serial();                                      <- Calls destructor!       throw("ERROR: Could not set com port time-outs");     }      // set DCB; disabling harware flow control; setting 1N8 mode     memset(&dcb, 0, sizeof(dcb));     dcb.DCBlength = sizeof(dcb);     dcb.BaudRate = bitRate;     dcb.fBinary = 1;     dcb.fDtrControl = DTR_CONTROL_DISABLE;     dcb.fRtsControl = RTS_CONTROL_DISABLE;     dcb.Parity = NOPARITY;     dcb.StopBits = ONESTOPBIT;     dcb.ByteSize = 8;      if(!SetCommState(commHandle, &dcb)) {       Serial::~Serial();                                    <- Calls destructor!       throw("ERROR: Could not set com port parameters");     }   }    if(cycleDtrOnStartup) {     if(!EscapeCommFunction(commHandle, CLRDTR))       throw("ERROR: clearing DTR");     Sleep(200);     if(!EscapeCommFunction(commHandle, SETDTR))       throw("ERROR: setting DTR");   }    if(testOnStartup) {     DWORD numWritten;     char init[] = "PJON-python init";     if(!WriteFile(commHandle, init, sizeof(init), &numWritten, NULL))       throw("writing initial data to port failed");     if(numWritten != sizeof(init))       throw("ERROR: not all test data written to port");   } };  Serial::~Serial() {   CloseHandle(commHandle); };  // and there is more etc ....... // ............. 

Next question, what will actually happen in memory when executing this code and it calls the destructor? I am not able to execute it and debug it.


This code is ugly but legal. When an exception is thrown from constructor, the corresponding destructor is never called. So calling it manually before throwing is needed to prevent the resource leak. The real bug here is not calling destructor manually in other cases before throwing exception.

Of course, a better way of doing this is having a separate RAII object that encapsulates commHandle. A unique_ptr with custom deleter can serve this role.

Any destructor beyond low-level libraries is a code smell in modern C++.


:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen: