Thread: This works, could it be better designed?

Hi guys.

I have finished one of the pointer exercises in my book and it works perfectly and I get the correct outputs, but could I have written it anymore efficiently? I have to use a struct with functions as shown below. I had to pass the object to the function which I did also. I know using a class would of been better but I was told to use a struct.

Here is my code:

Code:

#include <iostream>

// struct definition
struct Time
{
   int m_Hour;
   int m_Minute;
   int m_Second;

   // struct functions
   void setTime( Time *t );
   void getTime( Time *t );
};

// function to input the tume by user
void Time::setTime ( Time *t )
{
   std::cout << "Enter the hour: ";
   std::cin >> m_Hour;
   std::cout << "\nEnter the minutes: ";
   std::cin >> m_Minute;
   std::cout << "\nEnter the seconds: ";
   std::cin >> m_Second;
}

// function to display the time
void Time::getTime( Time *t )
{
   std::cout << "Time is: " << m_Hour << " :: " << m_Minute << " :: "
             << m_Second << std::endl;
}

// main function - driver //////////////////////////////////////////////////////
//
int main(int argc, char *argv[])
{
   Time *t = new Time;

   t->setTime( t );
   t->getTime( t );

   delete t;
   t = NULL;

   std::cin.get(); // freeze console output window
   std::cin.ignore();

   return 0; // return value from int main
}

Firstly, setTime() and getTime() need not be member functions. Alternatively, they can be member functions, but then they do not take any arguments but instead work on the *this object.

Next, you do not actually need to use new/delete:

Code:

int main(int argc, char *argv[])
{
   Time t;

   setTime( &t ); // or t.setTime(); if it is a member function
   getTime( &t ); // or t.getTime(); if it is a member function

   std::cin.get(); // freeze console output window
   std::cin.ignore();

   return 0; // return value from int main
}

Of course, the only use of pointers here is to simulate pass by reference, but since C++ has pass by reference, you could just use that instead.

Thank you laserlight. All taken onboard

I think you also forgot const correctness - those functions could take a const pointer since they never modify it!

I think you also forgot const correctness - those functions could take a const pointer since they never modify it!

I did forget about const correctness, but I would be more concerned about Time not being const in getTime(), and getTime() not being const if it was a member function. The const pointer thing would be moot if C++ references were used.

If references were used, then const references would have to be used...
getTime should probably be const too.

If references were used, then const references would have to be used...

For getTime() as a free function, but setTime() as a free function would still need to take by reference only. I suspect that making them member functions is not a good idea since they are more than simple setters/getters in that they actually perform I/O.

I think that the const pointer thing is not important since changing the pointer does not affect the caller. Changing what is pointed to does affect the caller, but this is guarded against by making Time const (or a const reference). As a side effect of using references, one automatically cannot change the reference (i.e., a reference is like a const pointer), so the implementation is also benefits.