Cpp track - Vehicle Purchase exercise - weak tests for 2nd requirement

Programming C++
1

[this may apply to other tracks, I noticed it in the cpp track]

In Vehicle Purchase the second requirement wants the solver to return the vehicle that comes first in lexicographical order. All the tests for this requirement work with an implementation that always returns the first parameter and never the second parameter - that is, the tests always pass in the options already sorted in lexicographical order.

I propose switching at least one of the tests so that the user has to evaluate the parameters. Such as:

TEST_CASE(“chooses Ford Focus over Ford Pinto”) {
std::string choice1{“Ford Pinto”};
std::string choice2{“Ford Focus”};
REQUIRE(vehicle_purchase::choose_vehicle(choice1, choice2) ==
“Ford Focus is clearly the better choice.”);
}

I’ll create a PR if anyone else thinks this is worth improving.

2

I do support the change. It is not some corner case, but a valuable addition.
The problem is, that this would retest all the instances that have been committed for this exercise. So it is a question of resources versus future application.

I don’t think too many people have solved the exercise for C++ yet, so it should be okay.

What do you think @iHiD

3

Up to today, only 267 had started this exercise, of which 259 had ‘completed’ it. (Source: C++ impact and status on Exercism)

So… 20 minutes of processing time?

I’d be interested to learn how much a single test run costs.

4

Thanks for checking! If it’s a valuable addition then it’s pretty much always worth the processing time, in my mind :slight_smile:

5

Please open a PR and ping me, so I can reopen it and assign some rep for it :slight_smile:

6

@vaeng Could you take a look at Vehicle Purchase - weak tests for 2nd requirement by pdmoore · Pull Request #1 · pdmoore/cpp · GitHub

I feel like I did something wrong this time, maybe applied changes to an older PR.

I can recreate if needed, let me know.

1 Like
7

Yes, we merged something else in the meantime, so it is not up to date. Also, your PR is on your own fork. I can make a new one for the exercism repository, but then I would not be able to review it.
The easiest path would be if you resubmitted your changes as a PR with the latest version on the exercism PR and gave me a ping.

8

@vaeng Yeah, makes sense - not running at full brainpower this past week :frowning:

Try this one- Update vehicle_purchase_test.cpp by pdmoore · Pull Request #2 · pdmoore/cpp · GitHub

9

If you have done a PR on the main cpp exercism branch correctly, you will find with a custom search for the main branch and you as an author.

So far there is nothing new there. Are you sure you made the PR to the correct repository?

It should look like this:

image
image1251×231 12.9 KB

You will find it under the github comparison for your branch and exercism main.

10

This shows two commits but it’s probably the same change committed twice :person_shrugging: