I’ve found this interview question at GeeksForGeeks and decided to actually code it in C++. The program currently prints the “value” of the horses (which is inaccessible by the coder so they can’t just look up who is number 1) and then proceeds to sort them and find the position of numbers 1,2 and 3 in a 5x5 matrix with the solution being a minimum of 7 races.
Here’s the Github repo: https://github.com/Shroomerian/HorseRacing
I saw that problem a few weeks ago on a youtube thumbnail, it’s quite a nice problem and I think it gets even more interesting if you start to consider 5^3 or even 5^N horses. Cool that you actually coded a solution for it! I like the idea of hiding the actual values to prevent the developer from cheating :)
I haven’t really checked whether you program does exactly what I’d expect it to do since I had a hard time to understand what’s happening. I think a few comments here and there would help to understand what exactly is going on and maybe some better names would hep, too (example:
number_matrix
- what does it do? I see from the type that it is a 2d array of ints but what does it contain and how does it relate tohorse_matrix
). I still have some general recommendations for you how you could improve your code:- You seem to use OS specific random numbers, there is a C++ api for that nowadays (
std::random_device
/std::mt19937
) - You call
rand() % 26
and check against 0, in that case you reroll - you could instead directly calculaterand() % 25 + 1
- As alternative to that manual
rand()
approach, you could look intostd::shuffle
- Take a look at sets (e.g.
std::unordered_set
), they have acontaints()
function that tells you whether a value is in the set or not. It’s not a big issue here but using vectors withstd::count
when all you want to do is check whether an element is in a set will not scale well - Consider using an enum instead of the
char option
. I’d go with something likeenum class Option { kRow, kColumn };
. If you want to stay with a char, I’d recommend not using a preprocessor directive for this () but instead go with a regular constant (e.g.
constexpr char kRow = 'r'
) - Consider using
std::unique_ptr<MatrixPosition>
instead of returning a raw pointer - that way you avoid memory leaks. ignore_count
andignore_vector
are only used in one function, so they don’t have to be members of the class.InitializeMatrix
probably does not need to be a class in the first place.- Consider not using
using namespace std
. For small projects that’s not an issue for bigger ones you’ll find your namespace polluted.
Something that you might also not be aware of is that
std::endl
automatically flushes buffers and makes sure that your line is printed immediately. If you don’t need this you can just use\n
instead which is a little bit faster. But that’s more of a “fun fact” than a general recommendation.Hope that helps and doesn’t sound too harsh, I know how code reviews can feel sometimes…
Thank you. Just the fact that you took all the effort into commenting is enough for me, thank you.
I didn’t really implement comments since I didn’t think it would expand that much (same goes for the namespace). As for some things which are unclear,
number_matrix
is a matrix I used to randomly generate the numbers that each horse would have to represent how fast they go, I didn’t know exactly how to create a random matrix where each number was unique using random, so I just added each previous number into a vector and checked if that number already exists there. The reason I did this (at the time) was because I REALLY didn’t want for the programmer to have access to the numbers, so I’d rather them being in a class that they can’t even access (protected constructor) and otherwise the numbers were hidden in the Horse private structure with the only way to interact with them being the two Race function (essentially sort functions)Also, I didn’t use sets, because I remember doing a problem in LeetCode and when I used them there the Memory part of the submission was so bad that it essentially said that it only beats 0.5%, when I replaced them with a vector for the same function the memory recuperated back to 56% (I know that really the difference is only a few KBs of memory, but it still traumatized me haha )
I’ll try to adress these issues and then I’ll try to make another release on Github, although you might think that might be a waste of time on such a problem, I have nothing better to do since it’s summer vacation.
I’m trying to learn programming, and it’s more of a hobby for now since I’m still not in University yet, although this year might be if they accept me (still waiting for admission results)
- You seem to use OS specific random numbers, there is a C++ api for that nowadays (
I really hate those kind of questions. They mostly have nothing to do with coding, and are really just a mathematical puzzle. Also most people are not able to figure those out on their own, and then everyone just ends up memorizing things.