Properly declaring a function using class members?

Question

I'm in the process of trying to convert a C program using structs into C++ with classes. I am trying to call a member function of a class B from within class A. Here is a snippet to explain my error:

    class Time{
        public:
        Time();
        Time(int hour, int minute);
        int time_Equal(Time &a, Time &b) const;
        -----
        private:
        int hour;
        int minute;
    };

    Boolean Time::time_Equal(Time &a, Time &b) const
    {
        /* If hour of Time "a" equals hour of Time "b" */
        if(a.hour == b.hour){
            if(a.minute == b.minute)
                    return TRUE;
        }
    }

    class DaTime{
        public:
        DaTime();
        DaTime(Day day, Time start, Time end);
        Boolean dt_Equal(DaTime &a, DaTime &b) const;

        private:
        int duration;
        Time start;
        Time end;
    };

    Boolean DaTime::dt_Equal(DaTime &a, DaTime &b) const
    {
            if(time_Equal(a.start, b.start)){
                   if(time_Equal(b.end, a.end))
                          return TRUE;
            }

            else
                    return FALSE;        
    }

The error I am getting is that time_Equal is not declared in this scope. The reason I do not understand this is because I have included a header file where Time is declared before DaTime. I don't see the issue with how I have declared the functions.

Answer 1

First, note that the boolean type is just bool.

Second, the return type of the time_Equal declaration is int, whereas the definition has Boolean. Pretty sure you want both to be bool. They should at least match.

Third, your time_Equal function does not modify its arguments, so you should make them const references (const Time &a and const Time &b).

Fourth, your time_Equal function is a non-static member of Time, but it doesn't use any of its members. That is, its output depends only on its arguments, and not on the object that this points at. This is a code smell. It means your function should probably be a static function or you should change the function to use this. So you have two options:

1. Keep it a member and change it to:

   bool time_Equal(const Time &other) const;
   

   And then in the implementation you should compare *this with the other argument. In this case, you would call it like a.start.time_Equal(b.start).

2. Make it either a static member or take it out of the class and make it a friend function and keep it with two arguments:

   bool time_Equal(const Time &a, const Time &b) const;
   

   In this case, you would call it like time_Equal(a.start, b.start).

Fifth, C++ has a built in operator to perform test for equality (==) which you can overload for a class. So you can in fact implement the above two options respectively like so:

bool operator==(const Time &other) const; // As a non-static member
// or
bool operator==(const Time &a, const Time &b) const; // As a static member or friend function

Then you can compare times like so: a.start == b.start. Nice, huh?

For more information, see the operator overloading FAQ.

Sixth, your time_Equal implementation does not return false when it should. You can easily rewrite the body as:

return a.hour == b.hour && a.minute == b.minute;

Answer 2

Function time_Equal is declared as a non-static member function of a class. So it can be used only with an instance of the class. Moreover it is declared as having parameters of type Time. However in function dt_Equal you call it passing arguments of type int

        if(time_Equal(a.start, b.start)){
               if(time_Equal(b.end, a.end))
                      return TRUE;
        }

You should declare function time_Equal either as a non-member function with two parameters or as member function with one parameter.

As a member function it could look as

bool Time::time_Equal( const Time &rhs ) const
{
    return (  hour == rhs.hour  && minute == rhs.minute );
}

As a non-member function it must be declared as a friend function of the class because data members hour and minute are defined as private Or if you have getters to hour and minute then there is no necessary to make it friend. In any case it could look as

bool Time::time_Equal( const Time &lhs, const Time &rhs )
{
    return (  lhs.hour == rhs.hour  && lhs.minute == rhs.minute );
    // or return (  lhs.get_hour() == rhs.get_hour()  && lhs.get_minute() == rhs.get_minute() );

}

Answer 3

time_Equal is a method in the Time class, yet you call it like it is a global function. An instance level method shall be called on an object.