ConfiguratorWindow::openDevice() requires refactoring

[samuelfmlourenco]

At the present time, inside "configuratorwindow.cpp", from lines 58 to 79, we have openDevice() implemented in the following manner:

// Opens the device and prepares the corresponding window
void ConfiguratorWindow::openDevice(quint16 vid, quint16 pid, const QString &serialstr)
{
    int err = cp2130_.open(vid, pid, serialstr);
    if (err == CP2130::ERROR_INIT) {  // Failed to initialize libusb
        QMessageBox::critical(this, tr("Critical Error"), tr("Could not initialize libusb.\n\nThis is a critical error and execution will be aborted."));
        exit(EXIT_FAILURE);  // This error is critical because libusb failed to initialize
    } else if (err == CP2130::ERROR_NOT_FOUND) {  // Failed to find device
        QMessageBox::critical(this, tr("Error"), tr("Could not find device."));
        this->deleteLater();  // Close window after the subsequent show() call
    } else if (err == CP2130::ERROR_BUSY) {  // Failed to claim interface
        QMessageBox::critical(this, tr("Error"), tr("Device is currently unavailable.\n\nPlease confirm that the device is not in use."));
        this->deleteLater();  // Close window after the subsequent show() call
    } else {
        vid_ = vid;  // Pass VID
        pid_ = pid;  // and PID
        serialstr_ = serialstr;  // and the serial number as well
        readDeviceConfiguration();
        this->setWindowTitle(tr("CP2130 Device (S/N: %1)").arg(serialstr_));
        displayConfiguration(deviceConfig_);
    }
}

The previous code was written assuming that if CP2130::open() doesn't return an error condition, then it must be OK to open the device. At the current time, CP2130::open() can only return CP2130::SUCCESS, CP2130::ERROR_INIT, CP2130::ERROR_NOT_FOUND and CP2130::ERROR_BUSY, and since no further expansions are planned, this assumption is right.

Nevertheless, the previous function should only assume success if and only if CP2130::SUCCESS is returned, instead of accepting anything different than CP2130::ERROR_INIT, CP2130::ERROR_NOT_FOUND or CP2130::ERROR_BUSY. Thus, openDevice() should be refactored:

// Opens the device and prepares the corresponding window
void ConfiguratorWindow::openDevice(quint16 vid, quint16 pid, const QString &serialstr)
{
    int err = cp2130_.open(vid, pid, serialstr);
    if (err == CP2130::SUCCESS) {  // Device was successfully opened
        vid_ = vid;  // Pass VID
        pid_ = pid;  // and PID
        serialstr_ = serialstr;  // and the serial number as well
        readDeviceConfiguration();
        this->setWindowTitle(tr("CP2130 Device (S/N: %1)").arg(serialstr_));
        displayConfiguration(deviceConfig_);
    } else if (err == CP2130::ERROR_INIT) {  // Failed to initialize libusb
        QMessageBox::critical(this, tr("Critical Error"), tr("Could not initialize libusb.\n\nThis is a critical error and execution will be aborted."));
        exit(EXIT_FAILURE);  // This error is critical because libusb failed to initialize
    } else {
        if (err == CP2130::ERROR_NOT_FOUND) {  // Failed to find device
            QMessageBox::critical(this, tr("Error"), tr("Could not find device."));
        } else if (err == CP2130::ERROR_BUSY) {  // Failed to claim interface
            QMessageBox::critical(this, tr("Error"), tr("Device is currently unavailable.\n\nPlease confirm that the device is not in use."));
        }
        this->deleteLater();  // Close window after the subsequent show() call
    }
}

Although, technically, this refactoring doesn't add value, it declares better the intention. At the cost of one extra line, it also accounts for future possibilities, assuming that CP2130::open() may be changed to return any other error condition, different from the ones currently returned. In that case, instead of trying to operate the device as if it was successfully opened, this new approach will simply not open the window, albeit without returning any error message. It accounts for future changes, without necessarily acknowledging them.

[samuelfmlourenco]

Code refactored in version 1.4.