Skip to content

Improvement expected #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improvement expected #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
374df17
Replace union by variant, and refactorization
MimiXav Dec 17, 2021
7e68de0
Removing m_error and replaced by variant methods
MimiXav Dec 17, 2021
aaacd7f
Factorization Expected<void, ErrorT> by isValid
MimiXav Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 51 additions & 81 deletions fty/expected.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <fmt/format.h>
#include <optional>
#include <string>
#include <type_traits>
#include <variant>

namespace fty {

Expand All @@ -33,55 +35,56 @@ template <typename T, typename ErrorT = std::string>
class Expected
{
public:
constexpr Expected() = delete;
constexpr Expected() = default;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, you cant to have default ctor

constexpr Expected(const T& value) noexcept;
constexpr Expected(T&& value) noexcept;
constexpr Expected(Expected&& other) noexcept;
template <typename UnErrorT>
constexpr Expected(Unexpected<UnErrorT>&& unex) noexcept;
template <typename UnErrorT>
constexpr Expected(const Unexpected<UnErrorT>& unex) noexcept;
~Expected();
~Expected() = default;

Expected(const Expected&) = delete;
Expected& operator=(const Expected&) = delete;
Expected(const Expected& other) = default;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, no copy here

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why should copy not be allowed?

Expected& operator=(const Expected& other) = default;

constexpr const T& value() const& noexcept;
constexpr T& value() & noexcept;
constexpr const T&& value() const&& noexcept;
constexpr T&& value() && noexcept;
constexpr const ErrorT& error() const& noexcept;
constexpr const T& value() const&;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove noexcept?

constexpr T& value() &;
constexpr const T&& value() const&&;
constexpr T&& value() &&;
constexpr const ErrorT& error() const&;

constexpr bool isValid() const noexcept;
explicit constexpr operator bool() const noexcept;

constexpr const T& operator*() const& noexcept;
constexpr T& operator*() & noexcept;
constexpr const T&& operator*() const&& noexcept;
constexpr T&& operator*() && noexcept;
constexpr const T& operator*() const&;
constexpr T& operator*() &;
constexpr const T&& operator*() const&&;
constexpr T&& operator*() &&;
constexpr const T* operator->() const noexcept;
constexpr T* operator->() noexcept;

private:
union {
T m_value;
ErrorT m_error;
};
bool m_isError = false;
std::variant<T, ErrorT> m_value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not union? this is more efficient type here

};

template <typename ErrorT>
class Expected<void, ErrorT>
{
public:
Expected() noexcept;
Expected() noexcept = default ;
template <typename UnErrorT>
Expected(Unexpected<UnErrorT>&& unex) noexcept;
template <typename UnErrorT>
Expected(const Unexpected<UnErrorT>& unex) noexcept;

Expected(const Expected&) = delete;
Expected& operator=(const Expected&) = delete;
Expected(const Expected& other)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No copy at all

{
m_error = std::move(other.error());
m_isError = other.m_isError;
};

Expected& operator=(const Expected&) = default;

constexpr bool isValid() const noexcept;
constexpr operator bool() const noexcept;
Expand Down Expand Up @@ -150,137 +153,104 @@ constexpr Expected<T, ErrorT>::Expected(T&& value) noexcept
template <typename T, typename ErrorT>
constexpr Expected<T, ErrorT>::Expected(Expected&& other) noexcept
{
if (other.m_isError) {
m_isError = true;
m_error = std::move(other.m_error);
} else {
m_value = std::move(other.m_value);
}
m_value = std::move(other.m_value);
}

template <typename T, typename ErrorT>
template <typename UnErrorT>
constexpr Expected<T, ErrorT>::Expected(Unexpected<UnErrorT>&& unex) noexcept
: m_error(std::move(unex.message))
, m_isError(true)
: m_value(std::move(unex.message))
{
}

template <typename T, typename ErrorT>
template <typename UnErrorT>
constexpr Expected<T, ErrorT>::Expected(const Unexpected<UnErrorT>& unex) noexcept
: m_error(unex.message)
, m_isError(true)
{
}

template <typename T, typename ErrorT>
Expected<T, ErrorT>::~Expected()
: m_value(unex.message)
{
if (m_isError) {
m_error.~ErrorT();
} else {
m_value.~T();
}
}

template <typename T, typename ErrorT>
constexpr const T& Expected<T, ErrorT>::value() const& noexcept
constexpr const T& Expected<T, ErrorT>::value() const&
{
assert(!m_isError);
return m_value;
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr T& Expected<T, ErrorT>::value() & noexcept
constexpr T& Expected<T, ErrorT>::value() &
{
assert(!m_isError);
return m_value;
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr const T&& Expected<T, ErrorT>::value() const&& noexcept
constexpr const T&& Expected<T, ErrorT>::value() const&&
{
assert(!m_isError);
return std::move(m_value);
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr T&& Expected<T, ErrorT>::value() && noexcept
constexpr T&& Expected<T, ErrorT>::value() &&
{
assert(!m_isError);
return std::move(m_value);
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr const ErrorT& Expected<T, ErrorT>::error() const& noexcept
constexpr const ErrorT& Expected<T, ErrorT>::error() const&
{
assert(m_isError);
return m_error;
assert(std::holds_alternative<ErrorT>(m_value));
return std::get<ErrorT>(m_value);
}

template <typename T, typename ErrorT>
constexpr bool Expected<T, ErrorT>::isValid() const noexcept
{
return !m_isError;
return std::holds_alternative<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr Expected<T, ErrorT>::operator bool() const noexcept
{
return !m_isError;
return isValid();
}

template <typename T, typename ErrorT>
constexpr const T& Expected<T, ErrorT>::operator*() const& noexcept
constexpr const T& Expected<T, ErrorT>::operator*() const&
{
assert(!m_isError);
return m_value;
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr T& Expected<T, ErrorT>::operator*() & noexcept
constexpr T& Expected<T, ErrorT>::operator*() &
{
assert(!m_isError);
return m_value;
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr const T&& Expected<T, ErrorT>::operator*() const&& noexcept
constexpr const T&& Expected<T, ErrorT>::operator*() const&&
{
assert(!m_isError);
return std::move(m_value);
return std::get<T>(m_value);
}

template <typename T, typename ErrorT>
constexpr T&& Expected<T, ErrorT>::operator*() && noexcept
constexpr T&& Expected<T, ErrorT>::operator*() &&
{
assert(!m_isError);
return std::move(m_value);
return std::get<T>(m_value);
}


template <typename T, typename ErrorT>
constexpr const T* Expected<T, ErrorT>::operator->() const noexcept
{
assert(!m_isError);
return &m_value;
return std::get_if<T>(&m_value);
}

template <typename T, typename ErrorT>
constexpr T* Expected<T, ErrorT>::operator->() noexcept
{
assert(!m_isError);
return &m_value;
return std::get_if<T>(&m_value);
}

// ===========================================================================================================

template <typename ErrorT>
inline Expected<void, ErrorT>::Expected() noexcept
{
}

template <typename ErrorT>
template <typename UnErrorT>
inline Expected<void, ErrorT>::Expected(Unexpected<UnErrorT>&& unex) noexcept
Expand All @@ -306,7 +276,7 @@ inline constexpr bool Expected<void, ErrorT>::isValid() const noexcept
template <typename ErrorT>
inline constexpr Expected<void, ErrorT>::operator bool() const noexcept
{
return !m_isError;
return isValid();
}

template <typename ErrorT>
Expand Down
65 changes: 64 additions & 1 deletion test/expected.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
*/
#include "fty/expected.h"
#include <catch2/catch.hpp>
#include <iostream>
#include <map>

struct St
{
St() = default;
St(const St&) = delete;
St& operator=(const St&) = delete;
St& operator=(const St&) = default;
St(St&&) = default;

bool func()
Expand All @@ -29,6 +31,21 @@ struct St
}
};

struct Msg
{
Msg() = default;
Msg(const Msg&) = default;
Msg& operator=(const Msg&) = default;
Msg(Msg&&) = default;

size_t size()
{
return m_data.size();
}

std::map<std::string, std::string> m_data;
};

TEST_CASE("Expected")
{
SECTION("Expected")
Expand Down Expand Up @@ -185,4 +202,50 @@ TEST_CASE("Expected")

REQUIRE_THROWS_AS(rethrow_exception(resTriggeredError.error()), std::runtime_error);
}

SECTION("Tests asign operator")
{
fty::Expected<void> result;
fty::Expected<void> result2;
fty::Expected<void> result3;
auto it = fty::Expected<void>();
CHECK(it);

auto func = []() -> fty::Expected<void> {
return {};
};
auto func2 = []() -> fty::Expected<void> {
return fty::unexpected("some error");
};
result = func();
CHECK(result);
result2 = result;
CHECK(result2);
result3 = func2();
CHECK(!result3);
CHECK("some error" == result3.error());
}

SECTION("Tests asign operator")
{
auto func = []() -> fty::Expected<Msg> {
Msg msg;
msg.m_data.emplace("keyMsg", "valueMsg");
return msg;
};

auto funcUnexpec = []() -> fty::Expected<Msg> {
return fty::unexpected("wrong");
};

fty::Expected<Msg> msg;
msg = func();
CHECK(msg->size() == 1);
CHECK((*msg).size() == 1);

fty::Expected<Msg> unexpec;
unexpec = funcUnexpec();
CHECK(!unexpec);
CHECK("wrong" == unexpec.error());
}
}