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

Page 626: Unit test for minmax function fails. #138

Closed
WraithGlade opened this issue Jan 21, 2020 · 1 comment
Closed

Page 626: Unit test for minmax function fails. #138

WraithGlade opened this issue Jan 21, 2020 · 1 comment

Comments

@WraithGlade
Copy link

@WraithGlade WraithGlade commented Jan 21, 2020

However, after I changed the test to the following code then it passed:

TEST_CASE("max and min")
{
	using namespace std::literals;
	auto length_compare = [](const auto& x1, const auto& x2) 
	{ 
		return x1.length() < x2.length(); 
	};

	REQUIRE(min("undiscriminativeness"s, "vermin"s, length_compare) == "vermin");

	REQUIRE(max("maxim"s, "ultramaximal"s, length_compare) == "ultramaximal");

	const string str1 = "minimaxes"s;
	const string str2 = "maximin"s;
	const auto result = minmax(str1, str2, length_compare);
	REQUIRE(result.first == "maximin");
	REQUIRE(result.second == "minimaxes");
}

Notice the main change:

	const string str1 = "minimaxes"s;
	const string str2 = "maximin"s;
	const auto result = minmax(str1, str2, length_compare);

It seems to not like the literal string values "minimaxes"s and "maximin"s and wants them to be assigned to variables. Hence the above code change.

This seems a bit odd to me (the fact that it won't accept literals etc). I'm not sure why it's designed to behave this way.

(PS: I'm using C++17 in Visual Studio 2019, using Microsoft's compiler, not clang, in case that's relevant.)

@JLospinoso

This comment has been minimized.

Copy link
Owner

@JLospinoso JLospinoso commented Jan 21, 2020

Thanks, @WraithGlade. It's failing because the book's example has undefined behavior!

I overlooked that these min/max functions return const & to their arguments. Passing in temporaries like I did is very bad. I'll add the errata.

JLospinoso added a commit that referenced this issue Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.