I have not checked your code for correct output, just for coding-related issues. Here are some comments.
Avoid using std
Pre-allocate vector size when you know roughly how large it will be
- Rather than rushing a number of
push_back
operations when you know how many, or roughly how many, you will need, preallocate the vector first. InnewStatesSet
you know the size you will makevector newStates
so you should callreserve
after the declaration and before you start yourpush_backs
. Otherwise the vector likely has to relocate after the first 2 pushes (std::vector
tends to double each time memory must be reallocated, so you'll usually go from 2 to 4 to 8 and so on....)
Brevity
You can shorten
checkGoal
like so:bool checkGoal(pair<int,int> x,int sum){ return (x.second==sum); }
Also consider restructuring the arguments to take
x.second
directly as anint
value parameter rather than unnecessarily passing the wholepair
. Both these adjustments should improve performance as well as code readability.
Avoid side effects
reverseTraversal
is a void that makes some changes to state and also prints something out. You generally want to do one or the other in a function. It's best not to have print statements scattered throughout functions.
Additional Possible improvements:
Reference to const can be better than passing by value
- Since you don't seem to manipulate your input parameter in
newStatesSet
, you should consider passing it by reference to const rather than by valuereference to const rather than by value. I may be wrong that this would improve performance since apair
ofints
shouldn't be that large, but you can benchmark to confirm.
Try making local copies of oft-used class members
- Looking at
newStatesSet
, I think you might consider making a local variable copy ofstate.first
andstate.second
since you access them often. Whether this improves performance is compiler dependentcompiler dependent.