idanshmu
idanshmu

Reputation: 5251

Am I abusing std::optional

How std::optional fits in my code?

My code contains MANY functions that looks roughly like:

bool GetName(_Out_ string& strRes)
{
   // try to get/calculate the value
   // value may also be empty
   // return true on success, false otherwise.
}

Now that I have found out std::optional, I simply code:

std::optional<std::string> GetName()

Why or when not to use std::optional?

  1. I understand that using std::optional comes with a performance price and for the sake of argument, let's assume I'm willing to pay it.
  2. I also understand that coding something like: std::optional<int> iVal = 9; is pointless.

Am I overly charmed by std::optional?

I find std::optional so natural that it lead me to a conclusion that, by default, many of my native types, bool, int, string, should be declared as std::optional.

So, instead of following the premise of:

Use std::optional only when absolutely needed

I'm leaning towards:

Use std::optional always unless you are sure it isn't needed now or in the future.

Question 1

Is the following rule valid:

use std::optional when:

  1. a value is calculated on run time
  2. calculation may fail
  3. you want to know if calculation failed

Question 2

If using std::optional becomes abundant, am I risking code readability issues?


Update

Answers and comments steered the discussion a bit away from my initial concern which is an attempt to define a rule of thumb for when to use std::optional.

I will address some of the common feedback I got:

Your are coding C-style error checking functions.

use exceptions when the calculation fails.

It is perhaps my fault for providing a not-so-good example of my motivation. GetName() is just a taste of my common use case. Instead of focusing on how errors are reflected (if any), I wanted to focus on is name a good candidate for optional?

I didn't state that if GetName returns false it implies some sort of error in my flow. When GetName return false, I could either generate a random name or just ignore the current iteration. Regardless of how the callers reacts to the return value, name is optional in a sense that it may not be present.

I can provide a better example:

void GetValuesFromDB(vector<string> vecKeys, map<string,string> mapKeyValue);

in this function I'm "forced" to provide two containers since I want to distinguish between:

  1. key was found in DB and it is empty. this key will be in mapKeyValue
  2. key was not found in DB. . this key will NOT be in mapKeyValue

But with optional I could go:

void GetValuesFromDB(map<string,std::optional<string> > mapKeyValue);

I believe that we're taking the word optional too literally.

I really support the notion that one may use std::optional If you want to represent a nullable type nicely. Rather than using unique values (like -1, nullptr, NO_VALUE or something)

The c++ standard committee could have easily decided to name std::optional, std::nullable.

Upvotes: 9

Views: 9656

Answers (4)

darune
darune

Reputation: 10962

I will try to make a short answer:

At it's core a std::optional is just a value with a bool (ala. std::pair<T, bool>). Remember that a raw pointer (in most contexts) is also optional.

In general use the following rule of thumb:

  1. Use exceptions for errors/failstates/exceptional cases/etc.
  2. Use only optional when needed ! Unless you are a "2nd or 3rd party" library writer, don't plan to futureproof with optionals.
  3. If you expect a result, don't use optional.
  4. Use optional when something is actually optionally - eg. a function that could end up finding 0 or 1 results it would make sense to return an optional. A function that could find multiple should naturally gravitate towards returning a container of results.
  5. Concerning strings, sometimes using an empty string is prefered (mainly due to simplicity) instead of std::optional<std::string> and sometimes the reverse is true.

Upvotes: 2

Useless
Useless

Reputation: 67723

Am I abusing std::optional

Maybe.

The intended semantics of std::optional are to indicate that something is optional. That is, that if the thing is not present, that is OK and not an error.

If your method fails, it returns an empty optional to indicate failure. But whether this failure is an also an error depends on the semantics expected by the calling code, which you haven't shown or described: what do you do if the optional result is empty?

For example,

auto name = GetName();
if (!name) { diediedie(FATAL_ERROR); }

suggests that the name isn't really optional. If you cannot (as you say) use exceptions here, the variant<string, error> approach seems clearer, and also allows your failing function to communicate the failure reason that would otherwise go in an exception.

Conversely

auto name = GetName();
if (name) {
  cout << "User is called " << *name << '\n';
} else {
  cout << "User is anonymous\n";
}

seems reasonable, because failure to fetch the name is not an error.


Why ... not to use std::optional?

When you don't wish to communicate that something is optional.

Obviously you can abuse it, like anything else, but it's likely to be confusing, misleading, unexpected and otherwise bad practise.

Am I overly charmed ... ?

Maybe. If your error handling contaminates all your code, you should probably consider failing earlier, rather than stumbling on with logic that can never succeed.

If you genuinely have lots of variables that may legally be missing - in terms of your program logic - then you're probably using it correctly.

Question 1

Again, it depends on what failure means in the context of your program.

Question 2

Yes, your code will be an unreadable mess if everything is optional, partly because of the visual noise, but mostly because you can never be sure which variables even exist at a given point in your logic.


Your GetValuesFromDB edit is probably fine to use optional if your database is relatively unstructured and has no constraints. You can't depend on any particular column being populated, everything is optional, you need to manually check for the presence of every field. They're genuinely optional.

If your DB is highly structured, it may not be appropriate to use optional for, say, the primary key. You cannot have a row without a primary key, so making it optional seems a bit weird. Maybe it's just not worth having a separate interface for the few columns that are mandatory, though.

I believe that we're taking the word optional too literally.

I genuinely can't tell if you even read my answer - your comment suggests you didn't understand it, at least.

Let's say we call it nullable instead. I would still suggest that returning nullable<T> for something that may legitimately not exist is fine, but returning the same type for something whose absence indicates an error, is wrong.

You have to decide what constitutes an error for your code. We can't do it for you, especially with the information provided. So should you use optional for everything? Maybe. Which is what I said. Decide on the semantics and preconditions of your own code, and you'll know which things are optional, which things are mandatory and whose absence should be handled as an error instead of a regular code path.

Upvotes: 8

Radosław Cybulski
Radosław Cybulski

Reputation: 2992

std::optional is a class, that represents "optional" value. If you have a case, where you might produce a value or you might not, then optional is a way to go. Performance price is negligible (the additional condition, if the value is here might actually slow things down, but since value is optional, you will always somewhere have to check, if value is present anyway), the real price is memory:

printf("%d\n", (int)sizeof(std::string));
printf("%d\n", (int)sizeof(std::optional<std::string>));

prints:

32
40

on clang 64 bit.

So if you do in - let's say a structure:

struct A {
    std::optional<std::string> a, b, c, d, e, f, g, h;
};

struct B {
    std::string a, b, c, d, e, f, g, h;
    unsigned char has_a : 1,
        has_b : 1,
        has_c : 1,
        has_d : 1,
        has_e : 1,
        has_f : 1,
        has_g : 1,
        has_h : 1;
};

then you will get sizeofs:

A - 320
B - 264

which might actually hurt. But usually don't.

So are you abusing optional? If you use optional to signal an error, then probably slightly yes, depending on error's context. But it always depends. If it's more important for code clarity to signal, that function didn't produce value instead of why it didn't produce value, then sure, go for it.

Upvotes: 2

Bathsheba
Bathsheba

Reputation: 234665

I'd say this is an abuse.

I'd rewrite to std::string getName() const (relying on the now-required RVO) and if something untoward happens (e.g. some kind of fetching error), then throw an exception (of a custom type derived from std::exception). Whether or not a blank name is an error is a matter for your application.

If you are not allowed to use exceptions, then rely on C style error propagation of the form

int getName(std::string*) const

as using std::optional makes the actual error opaque (unless you imbue an error struct onto the current thread).

Upvotes: -4

Related Questions