7
\$\begingroup\$

Design a Tips class that calculates the gratuity on a restaurant meal.

Its only class member variable, taxRate, should be set by a one-parameter constructor to whatever rate is passed to it when a Tips object is created. If no argument is passed, a default tax rate of .065 should be used.

The class should have just one public function, computeTip. This function needs to accept two arguments, the total bill amount and the tip rate. It should use this information to compute what the cost of the meal was before the tax was added. It should then apply the tip rate to just the meal cost portion of the bill to compute and return the tip amount.

Demonstrate the class by creating a program that creates a single Tips object, and then allows the program user to retrieve the correct tip amount using various bill totals and desired tip rates.

What can I do better?

#include<iostream>
#include <string>

using namespace std;

class Tips
{
       private :
           double taxRate;

       public:
          Tips()
          {
                taxRate = .65;
                cout << "Tax rate is "<<taxRate<<endl;
          }

          Tips(double tax)
          {
                     taxRate = tax;
                     cout << "Tax rate is"<<taxRate<<endl;
          }

          double computeTip(double billAmount,double tipRate)
          {
                double mealCost = billAmount - billAmount*(taxRate);

                cout << "Meal cost is with no tax "<<mealCost<<endl;
                return mealCost+tipRate;
          }
};

int main()
{
    Tips tip ;
    cout << "Meal cost + tip rate is "<<tip.computeTip(100,20)<<endl;

    return 0;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ A tax rate of 65% seems steep. taxRate = .65; \$\endgroup\$ – Martin York Nov 18 '14 at 21:24
  • 1
    \$\begingroup\$ Also, if it's a tip, then it should be tipRate rather than taxRate. \$\endgroup\$ – Edward Nov 19 '14 at 2:36
  • \$\begingroup\$ Be careful with the implicit constructor! Consider making it explicit. \$\endgroup\$ – Michael Urman Nov 20 '14 at 0:55
6
\$\begingroup\$

Inconsistant layout:

#include<iostream>   // No space
#include <string>    // Space

Be consistent in your code. I prefer the version with the space.

Using namespace std

Arrrrr. Don't do that. see: Why is “using namespace std;” considered bad practice?.

Definately never do it in a header file. You are pulliting the namesapce for anybody that includes your code (which can change behavior) and break other peoples code. Don't do it in source files because of the reasons you read in the linked article.

Default argument

      Tips()
      {
            taxRate = .65;
            cout << "Tax rate is "<<taxRate<<endl;
      }

      Tips(double tax)
      {
                 taxRate = tax;
                 cout << "Tax rate is"<<taxRate<<endl;
      }

When you see code like this you can usually replace it with a single constructor that has a parameter with a default argument (as they are basically doing the same thing).

      Tips(double tax = 0.065) // Corrected you constant.
      {
           taxRate = tax;
           cout << "Tax rate is"<<taxRate<<endl;
      }

Be consistent with spacing (white space is your friend)

           cout << "Tax rate is"<<taxRate<<endl;
                ///            ^^^^^^  ^^^^^        Looking very tight here.

           cout << "Meal cost + tip rate is "<<tip.computeTip(100,20)<<endl;
                ////                        ^^^^^                   ^^^^

Prefer '\n' over std::endl

std::end forces a flush of the stream. This is never what you actually want. The automated flushing of the stream will work much better than manual flushing.

Prefer to use the initializer list.

      Tips(double tax = 0.065) // Corrected you constant.
           :   taxRate(tax)
      {
           cout << "Tax rate is"<< taxRate << endl;
      }

Arithmetic issue

If the billAmount is mealCost * (1 + taxRate) then this is not the formula to calculate meal cost.

            double mealCost = billAmount - billAmount*(taxRate);
\$\endgroup\$
  • 2
    \$\begingroup\$ I'd suggest that it would also be better to omit I/O from the class and to have all printing be done by driving code (if needed). This suggests the utility of method to fetch the current tip rate, but unfortunately, the specification seems to prohibit that. \$\endgroup\$ – Edward Nov 19 '14 at 2:35
4
\$\begingroup\$
  • Just a tiny nitpick, but this inconsistency is a little tacky:

    #include<iostream>
    #include <string>
    

    The one with spacing is preferred for all.

  • @Abbas' suggestions are okay, but you can use an initializer list instead for the constructor that takes an argument:

    Tips(double tax) : taxRate(tax) {}
    

    Also regarding that answer, I don't think an Init() function would be necessary. Plus, as it's initializing something, it shouldn't be displaying anything (unless it's solely for testing). That also goes for your original constructor. This initializer list is all that you should need, but you may still have a default as well.

    Regarding the displays, just leave them all out the class. It should only be done in main or some other non-member function. These class functions already have a purpose, neither of which involve displaying anything.

  • You're using std::endl quite a bit, but you don't really need it at all. It also flushes the buffer, which is unnecessary here, and it also slower. Instead, just output a "\n" to get a newline by itself to avoid this flush. See this for more info on how they contrast.

  • It's unnecessary to have an explicit return 0 at the end of main(). Reaching this point already implies successful termination, so the compiler should do this same return for you.

\$\endgroup\$
0
\$\begingroup\$

You have double code in the constructors. Place this initialization code in a separate method and call that method from the constructor. Your code will look like following:

private:
    void Init(double tax)
    {
        taxRate = tax;
        cout << "Tax rate is: "<<taxRate<<endl;
    }

public:
    Tips()
    {
        Init(.65);
    }

    Tips(double tax)
    {
        Init(tax);
    }
\$\endgroup\$
  • 1
    \$\begingroup\$ Providing a default argument for tax and using a constructor initialization list would be the C++ way of doing this. \$\endgroup\$ – jliv902 Nov 18 '14 at 16:19
  • \$\begingroup\$ Don't need Init() constructors can call other constructors. Also much easier to solve with default parameters. \$\endgroup\$ – Martin York Nov 18 '14 at 23:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.