Skip to content
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.

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

Possibly revert #30 #31

Open
tmattha opened this issue Nov 12, 2019 · 0 comments
Open

Possibly revert #30 #31

tmattha opened this issue Nov 12, 2019 · 0 comments

Comments

@tmattha
Copy link

tmattha commented Nov 12, 2019

Hey neither developers, Hey @nikhedonia,

so I contributed #30 some time ago and promised to implement some move specializations for neither::Maybe which I just started. However, I was not a 100% convinced of my implementation for Eithers at that time and now ran into more issues with neither::Maybe. Mostly that the hasValue attribute needed to be mutable now.

My main concern is that pull #30 introduced mutations to Eithers, which are not directly visible to the developer. Given that a stored value is not copy-constructible the Either would lose its value on map & join. At that time I thought this was the pill one had to swallow to work with unique_ptrs in lvalues.

Now it dawned on me that you can always std::move the object to retrieve an rvalue again.
Here is a very minimal example:

TEST(neither, either_fromLvalueByMove) {
  neither::Either<std::unique_ptr<int>, int> e = left(std::make_unique<int>(0));
  std::move(e).leftMap([](auto ptr) {
    EXPECT_EQ(*ptr, 0);
    return std::move(ptr);
    });
}

From my POV this is much more clear as it shows clearly that e is potentially no longer usable after this operation. Therefore I would suggest to revert this pull and to just add some documentation on some common and niche use cases to the documentation.

Kind regards
Tilmann

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant