Function with a custom return type and the "false" return conditions?

I have a function that returns a custom class structure, but how should I handle the cases where I wish to inform the user that the function has failed, as in return false.

My function looks something like this:

Cell CSV::Find(std::string segment) {
  Cell result;
  // Search code here.
  return result;
}

So when succesful, it returns the proper result, but how should I handle the case when it could fail?

I thought about adding a boolean method inside Cell to check what ever Cell.data is empty or not (Cell.IsEmpty()). But am I thinking this issue in a way too complicated way?

Upvotes: 21

Views: 3420

Answers (6)

manlio
manlio

Reputation: 18972

A further option is using multiple return values:

std::pair<Cell, bool> CSV::Find(std::string segment) {
  Cell result;
  // Search code here.
  return {result, found};
}

// ...

auto cell = CSV::Find("foo");
if (cell->second)
  // do stuff with cell->first

The boolean flag says whether the requested Cell was found or not.

PROs
  • well known approach (e.g. std::map::insert);
  • quite direct: value and success indicator are return values of the function.
CONs
  • obscureness of first and second which requires to always remember the relative positions of values within the pairs. C++17 structured bindings / if statement with initializer partially resolve this issue:
    if (auto [result, found] = CSV::Find("foo"); found)
      // do stuff with `result`
    
  • possible loss of safety (the calling code has to check if there is a result value, before using it).

Details

Upvotes: 1

Matthieu M.
Matthieu M.

Reputation: 300439

For parsing, it is generally better to avoid std::string and instead use std::string_view; if C++17 is not available, minimally functional versions can be whipped up easily enough.

Furthermore, it is also important to track not only what was parsed but also the remainder.

There are two possibilities to track the remainder:

  • taking a mutable argument (by reference),
  • returning the remainder.

I personally prefer the latter, as in case of errors it guarantees that the caller has in its hands a unmodified value which is useful for error-reporting.


Then, you need to examine what potential errors can occur, and what recovery mechanisms you wish for. This will inform the design.

For example, if you wish to be able to parse ill-formed CSV documents, then it is reasonable that Cell be able to represent ill-formed CSV cells, in which case the interface is rather simple:

std::pair<Cell, std::string_view> consume_cell(std::string_view input) noexcept;

Where the function always advances and the Cell may contain either a proper cell, or an ill-formed one.

On the other hand, if you only wish to support well-formed CSV documents, then it is reasonable to signal errors via exceptions and that Cell only be able to hold actual cells:

std::pair<std::optional<Cell>, std::string_view> consume_cell(...);

And finally, you need to think about how to signal end of row conditions. It may a simple marker on Cell, though at this point I personally prefer to create an iterator as it presents a more natural interface since a row is a range of Cell.

The C++ interface for iterators is a bit clunky (as you need an "end", and the end is unknown before parsing), however I recommend sticking to it to be able to use the iterator with for loops. If you wish to depart from it, though, at least make it work easily with while, such as std::optional<Cell> cell; while ((cell = row.next())) { ... }.

Upvotes: 0

user743382
user743382

Reputation:

There are three general approaches:

  • Use exceptions. This is what's in Bathsheba's answer.
  • Return std::optional<Cell> (or some other type which may or may not hold an actual Cell).
  • Return bool, and add a Cell & parameter.

Which of these is best depends on how you intend this function to be used. If the primary use case is passing a valid segment, then by all means use exceptions.

If part of the design of this function is that it can be used to tell if a segment is valid, exceptions aren't appropriate, and my preferred choice would be std::optional<Cell>. This may not be available on your standard library implementation yet (it's a C++17 feature); if not, boost::optional<Cell> may be useful (as mentioned in Richard Hodges's answer).

In the comments, instead of std::optional<Cell>, user You suggested expected<Cell, error> (not standard C++, but proposed for a future standard, and implementable outside of the std namespace until then). This may be a good option to add some indication on why no Cell could be found for the segment parameter passed in, if there are multiple possible reasons.

The third option I include mainly for completeness. I do not recommend it. It's a popular and generally good pattern in other languages.

Upvotes: 43

Sebastian Stern
Sebastian Stern

Reputation: 642

If you can use C++17, another approach would be to use an std::optional type as your return value. That's a wrapper that may or may not contain a value. The caller can then check whether your function actually returned a value and handle the case where it didn't.

std::optional<Cell> CSV::Find(std::string segment) {
  Cell result;
  // Search code here.
  return result;
}

void clientCode() {
   auto cell = CSV::Find("foo");
   if (cell)
      // do stuff when found
   else
      // handle not found
}

Upvotes: 1

Richard Hodges
Richard Hodges

Reputation: 69942

Is this function a query, which could validly not find the cell, or is it an imperative, where the cell is expected to be found?

If the former, return an optional (or nullable pointer to) the cell.

If the latter, throw an exception if not found.

Former:

boost::optional<Cell> CSV::Find(std::string segment) {
  boost::optional<Cell> result;
  // Search code here.
  return result;
}

Latter: as you have it.

And of course there is the c++17 variant-based approach:

#include <variant>
#include <string>

struct CellNotFound {};
struct Cell {};

using CellFindResult = std::variant<CellNotFound, Cell>;


CellFindResult Find(std::string segment) {
  CellFindResult result { CellNotFound {} };

  // Search code here.
  return result;
}

template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; };
template<class... Ts> overloaded(Ts...) -> overloaded<Ts...>;

void cellsAndStuff()
{
    std::visit(overloaded
    {
        [&](CellNotFound)
        {
            // the not-found code
        },
        [&](Cell c)
        {
            // code on cell found
        }
    }, Find("foo"));
}

Upvotes: 7

Bathsheba
Bathsheba

Reputation: 234885

The C++ way of dealing with abject failures is to define an exception class of the form:

struct CSVException : std::exception{};

In your function you then throw one of those in the failure branch:

Cell CSV::Find(std::string segment) {
  Cell result;
  // Search code here.
  if (fail) throw CSVException();
  return result;
}

You then handle the fail case with a try catch block at the calling site.

If however the "fail" branch is normal behaviour (subjective indeed but only you can be the judge of normality), then do indeed imbue some kind of failure indicator inside Cell, or perhaps even change the return type to std::optional<Cell>.

Upvotes: 4

Related Questions