nayuki / QR-Code-generator

High-quality QR Code generator library in Java, TypeScript/JavaScript, Python, Rust, C++, C.
https://www.nayuki.io/page/qr-code-generator-library
5.33k stars 1.12k forks source link

C++: Default constructor for QrCode or no-throw versions of the factory functions #176

Closed jselbie closed 1 year ago

jselbie commented 1 year ago

Suggestion for C++. A default constructor that represents an empty or unassigned QrCode.

Useful for code paths that want to create a QrCode, but not have the entire function body wrapped with a try/catch block:

Take this as an example:

try
{
    QrCode qrc = qrcodegen::QrCode::encodeText(text, QrCode::Ecc::HIGH);
    // a bunch of rendering code that operates on qrc
    // ...
    // ...
    // ...
}
catch(logic_error ex)
{
    // handle failure
}

So a lot of code needs to be wrapped in a try catch block just for the lifetime of qrc.

This would be nice:

QrCode qrc; // can't declare this without a default constructor
try
{
    qrc = qrcodegen::QrCode::encodeText(text, QrCode::Ecc::HIGH);
}
catch(logic_error ex)
{
    // handle failure
}
// a bunch of rendering code that operates on qrc

Or just have the factory function signal an error or have a bit on the qrcode itself

qrc = qrcodegen::QrCode::encodeText(text, QrCode::Ecc::HIGH);
if (qrc.isValid()) {
    // a bunch of rendering code that operates on qrc
    // ...
    // ...
    // ...
} else {
    // handle failure
}

Or both. I wouldn't be surprised if the C++ purists disagreed. But in large code bases, pure C++ takes a backseat to a mix of C/C++ code. And not every code base and team deals with exceptions that well.

nayuki commented 1 year ago

Ooh, this is a tough one. Languages like Java and Rust have compile-time definite-assignment analysis, which sidesteps this C++ problem you highlighted.

I'll have to think about it. In the meantime, you can use std::optional in the top line.

jselbie commented 1 year ago

I submitted a PR for your consideration.

https://github.com/nayuki/QR-Code-generator/pull/177

nayuki commented 1 year ago

Thanks. Your pull request matches what I had in mind, which is to initialize the fields to zero or some default value. I have a lot of questions and concerns however.

You've bifurcated the state space of a QrCode object in C++. Previously, if you were given a fully constructed QrCode object, it was guaranteed to be valid and contain useful data. Now, you would have to call isValid(), and depending on the result, possibly refrain from calling getModule(), etc. Every method like getVersion() would need an additional sentence saying, "If getValue() is false, then this method returns a certain/implementation-defined/uninitialized value", and an additional burden is placed on the user.

I have trouble seeing how this is a useful code pattern:

QrCode qr;
try {
    qr = qrcodegen::QrCode::encodeText(...);
} catch (...) {
    return;
}
paint qr.getModule(x, y);

The QrCode object should only "exist" if it was successfully constructed, not in a wider scope. And it should only be destructed if it was successfully constructed. Having a placeholder value for before construction, and such that it has a no-op destruction, makes the design intent less clear.

Useful for code paths that want to create a QrCode, but not have the entire function body wrapped with a try/catch block: [...] So a lot of code needs to be wrapped in a try catch block just for the lifetime of qrc.

I can see two reasons why you might not want to wrap the entire function body in a try-catch block. One is that only the static factories can throw; all the getters are unconditionally safe. Two is that only wrapping the factory call means that the exception handler is much closer in the code.

I seriously suggest using std::optional to implement your use-case because it's more idiomatic, safer, and does not burden me or other users who don't want this feature. Pseudocode:

std::optional<QrCode> qr = std::nullopt;
try {
    qr = std::make_optional(qrcodegen::QrCode::encode_text(...));
} catch (...) { ... }
paint qr->getModule(x, y);

If you want to avoid std::optional but still only want to wrap the factory call in try-catch, then you would need a subsequent function:

void mainFunction() {
    try {
        QrCode qr = qrcodegen::QrCode::encodeText(...);
        continueProcessing(std::move(qr))
    } catch (...) { ... }
}

void continueProcessing(QrCode qr) {
    ...
}

You also mentioned the possibility of factory functions signalling an error. Again, I feel the correct way to do this is not to turn QrCode into an intrusive data structure that encodes multiple variant states, but to use a wrapper like std::variant in C++ (Result<T,E> in Rust) which has either a QrCode or an error value of an error type.

nayuki commented 1 year ago

Any thoughts before I close this?

jselbie commented 1 year ago

You can close.

On Mon, Mar 6, 2023 at 8:32 PM Nayuki @.***> wrote:

Any thoughts before I close this?

— Reply to this email directly, view it on GitHub https://github.com/nayuki/QR-Code-generator/issues/176#issuecomment-1457513154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHNSF3UT5GS2HDOZ4BEJH3W2226ZANCNFSM6AAAAAAVBGGWTE . You are receiving this because you authored the thread.Message ID: @.***>