An implementation of Uber's “Fare Estimator”

Question

CodeSignal put out a challenge by Uber that involves writing a "fare estimator" that involves a function dependent on cost per minute, cost per mile, ride time and ride distance.

The formula is along the lines of...

Given that: cₐ stands for cost per minute, rₐ stands for ride time, cₑ stands for cost per mile and rₑ ride distance, the estimation formula is: $$E = c_a \cdot r_a + c_e \cdot r_e $$

My solution:

/* CodeSignal Company Challenges
 *
 * A solution to Uber's "fareEstimator" challenge
 * By A. S. "Aleksey" Ahmann, [email protected]
 *
*/

double estimation_forumla(int ride_time, int ride_distance, double cost_per_minute, double cost_per_mile) {
    return (cost_per_minute * ride_time) + (cost_per_mile * ride_distance);
}

vector<double> fareEstimator(int ride_time, int ride_distance, vector<double> cost_per_minute, vector<double> cost_per_mile) {
    
    vector<double> estimates;
    for (int i = 0; i < cost_per_minute.size(); i++)
        estimates.insert(estimates.end(), estimation_forumla(ride_time, ride_distance, cost_per_minute[i], cost_per_mile[i]));
    
    return estimates;
}

I initially solved this in Python and ported it to C++ whilst taking into account the feedback of other programmers on that question.

They recommended not using any fancy code golf, as shorter code usually implies difficulty to maintain but does not imply faster. In C++, it's more difficult to "code golf" my solution, so no fancy tricks there (I hope). They also recommended that I use better names to my variables. Do you feel that I did a decent job naming my variables?

Finally, they recommended that I use type hints. I believe that the newer versions of C++ allow for type inference (though please correct me on that if I'm wrong :P), but I decided to declare variables by their type.

Do you feel that I was able to follow the advice given and did better than my initial attempt? Also, just give me any feedback that you have for my code. What could I have done better? What did you like (or dislike) about it? Let me know!

You can check out the full challenge (but you might need to sign up).

And here's my solution on GitHub if you're interested.

Answer 1

There isn't much code to review here.

1. Use correct spelling. estimation_forumla should be estimation_formula.

2. Use consistent naming conventions. estimation_forumla is snake case, while fareEstimator is camel case.

3. Pass vectors by const reference:

   vector<double> fareEstimator(int ride_time, int ride_distance, const vector<double>& cost_per_minute, const vector<double>& cost_per_mile)
   

   This avoids an unnecessary copy.

4. Instead of using insert with the end iterator, just use push_back to insert elements into the vector.

5. Since you know what the size of the resulting vector will be, you can reserve that memory in advance by calling estimates.reserve(cost_per_minute.size()). This will avoid any unnecessary resizing of the vector.

6. Use std::size_t instead of int for indexes.

Answer 2

It appears that there's a hidden using std::vector. In most cases, I think it's better to write the type name in full and omit the using. The namespace name std is very short! Definitely don't using namespace std - that's quite harmful.

Regardless, we need #include <vector> for this to compile on most systems - if it compiles for you without, then that may mislead you - the include is not optional!

This kind of code is very easy to unit test. You should include the tests with the code.

It's not clear from the description whether the interface is forced upon you. Two parallel vectors is a very poor choice - a vector of std::pair would be significantly better, though the ideal would be a vector of a struct that gives meaningful names to the two rates. If you're really stuck with the parallel vectors, it may be wise to anticipate being passed vectors of different lengths - either throw an exception in that case, or use the std::min() of the two lengths.

And all the things in Rish's answer.