Overloading operators in PhoneCall class

Question

I am trying to understand the concepts of friend functions, overloaded operators, and inheritance in C++. I am very confused on the specifics used for coding, since I am fairly new in programming, and working in the Visual Studio C++ environment in writing code.

The following is the project details for writing a program in Visual Studio C++:

Design a PhoneCall class that holds a phone number to which a call is placed, the length of the call in minutes, and the rate charged per minute. Overload extraction and insertion operators for the class. In this program, overload the == operator to compare two PhoneCalls. Consider one PhoneCall to be equal to another if both calls are placed to the same number. Also, create a main() function that allows you to enter ten PhoneCalls into an array. If a PhoneCall has already been placed to a number, do not allow a second PhoneCall to the same number.

I really need some feedback on the following code:

#include <iostream> 
#include <string> 

using namespace std; 

class PhoneCall { 
private: 
    string phonenumber; 
    double perminuterate; 
    double calldurationminutes; 
public: 
    bool operator==( const PhoneCall &n ) const; 
    friend ostream & operator<<( ostream &f, const PhoneCall &n ); 
    friend istream & operator>>( istream &f, PhoneCall &n ); 
 }; 
 bool PhoneCall::operator==( const PhoneCall &n ) const { 
    return phonenumber == n.phonenumber; 
 }; 
 ostream & operator<<( ostream &f, const PhoneCall &n ) { 
    f << "Phone number: " << n.phonenumber << 
         ", Duration: " << n.calldurationminutes << 
         " minutes, Rate: " << n.perminuterate << endl; 
 return f; 
 } 
 istream & operator>>( istream &f, PhoneCall &n ) { 
 f >> n.phonenumber; 
 f >> n.calldurationminutes; 
 f >> n.perminuterate; 
 return f; 
 } 

int main( ) { 
 PhoneCall a[10]; 
 cout << "Enter 10 phone numbers, duration in minutes, and the per-minute rates." << 
     endl << "Separate each with a space and then hit enter to complete it." << endl; 
 for ( int i= 0; i < 10; ) { 
     cin >> a[i]; 
     int j; 
     for ( j= 0; j < i; ++j ) 
         if ( a[i] == a[j] ) { 
             cout << "Duplicate number information ignored. Try again." << endl; 
             break; 
         } 
     if ( j == i ) ++i; 
 } 
 for ( int i= 0; i < 10; ++i ) 
     cout << a[i];

 system("pause");
 return 0; 
 } 

Answer 1

using namespace std; 

This is generally frowned upon, especially at the "global" level like this. Since you don't know what names might be defined inside namespace std, this has the potential to create conflicts with your code.

class PhoneCall { 
private: 

Although it's harmless, the private: here is redundant--members of a class are private by default.

    string phonenumber; 
    double perminuterate; 
    double calldurationminutes; 

I think I'd rename these to something like:

string phone_number;
double per_minute_rate;
double call_duration_minutes;

If at all possible, I'd use an std::vector instead of a "raw" array, so this:

 PhoneCall a[10]; 

...would turn into something like:

std::vector<PhoneCall> a(10);

std::endl is drastically overused. I'd generally just use '\n';. endl also flushes the file's buffer, which you rarely want. When you read/write cin and cout, flushes are done automatically between writing and reading anyway.

 cout << "Enter 10 phone numbers, duration in minutes, and the per-minute rates." << 
     endl << "Separate each with a space and then hit enter to complete it." << endl; 
 for ( int i= 0; i < 10; ) { 
     cin >> a[i]; 
     int j; 
     for ( j= 0; j < i; ++j ) 
         if ( a[i] == a[j] ) { 
             cout << "Duplicate number information ignored. Try again." << endl; 
             break; 
         } 
     if ( j == i ) ++i; 
 } 

You might want to use std::find to find whether the array already contains an instance of a particular phone number. This could simplify your logic quite a bit.

When you print out your phone call objects:

 for ( int i= 0; i < 10; ++i ) 
     cout << a[i];

...you traditionally want to only print out the data itself in your operator<<, and leave any final new-line to separate one object from the next to be printed somewhere else (like right here).

 system("pause");

I'd avoid this, and use something like:

std::cout << "Press return to quit."
char ch;
std::cin.get(ch);

Answer 2

Apart from Jerry's remarks, I would also provide an operator!= to go along with operator== (implementing the former in terms of the latter to avoid code duplication), and make both operators non-member friend functions.

One advantage is that non-member functions treat left/right arguments symmetrically and avoid tricky argument conversion ambiguities when you have single-argument constructors (not the case in this example, but it's a good habit to get into).

If you have to overload other operators, there is even a Boost.Operators library to help you do this automatically if you overload (overkill for this example, though).

Finally, I prefer writing ArgType const& argname because const applies right-to-left.

class PhoneCall 
{ 
public:
//....
    friend bool operator==(PhoneCall const& L, PhoneCall const& R)
    {
        return L.phone_number == R.phone_number;  
    }

    friend bool operator!=(PhoneCall const& L, PhoneCall const& R)
    {
        return !(L == R);
    }         
 };

Further reading: the Operator Overloading Q&A.