-2

This is my question:

(Enhancing Class Time) Provide a constructor that’s capable of using the current time from the time and localtime functions—declared in the C++ Standard Library header —to initialize an object of the Time class.

Here's my code: .h file

#ifndef TIME
#define TIME

class Time
{
public:
Time();
Time(int, int, int);
void Display();
private:
int hour, minute, second;
};
#endif // !1

.cpp file

#include "Time.h"
#include <ctime>
#include <iostream>


using namespace std;

Time::Time(){}
Time::Time(int h, int m, int s)
{
hour = h;
minute = m;
second = s;
time_t currenttime;
struct tm timeinfo;
time(&currenttime);
localtime_s(&timeinfo, &currenttime);

h = timeinfo.tm_hour;
m = timeinfo.tm_min;
s = timeinfo.tm_sec;
}

void Time::Display()
{
cout << hour << ":" << minute << ":" << second << endl;
}

main.cpp file

#include <iostream>
#include "Time.h"
#include <ctime>

int main()
{
    Time currentTime;

    currentTime.Display();

    system("pause");
    return 0;
}

The output:

-858993460:-858993460:-858993460

ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97
notacomputer
  • 13
  • 3
  • Take the code that reads the `time()` and move it into your default ctor. Except change the last 3 lines to assign the member variables. – 001 Nov 13 '18 at 18:31
  • Are negative values for h/m/s meaningful in any way? If you say 'yes', then int is fine, otherwise I'd prefer unsigned int to reflect the issue. Maybe you additionally want to check that h must be smaller than 24, m < 60 and s < 61 (if you want to support leap seconds, otherwise 60). Consider appropriate error handling (falling back to defaults, throwing an exception, ...). – Aconcagua Nov 13 '18 at 18:40
  • [std::chrono::system_clock::now](https://en.cppreference.com/w/cpp/chrono/system_clock/now) – Jesper Juhl Nov 13 '18 at 18:48

2 Answers2

1

your time is not initialized properly that is why you get those values...

and when you do 

Time currentTime;

you are creating the Time object using the default constructor leaving the fields uninitialized....

do something like

private:
int hour{0};
int minute{0};
int second{0};

another trick could be call the 2nd const from the default one because there is where you placed the logic to init the object...

Time::Time() : Time(0, 0, 0)
{}
ΦXocę 웃 Пepeúpa ツ
  • 47,427
  • 17
  • 69
  • 97
  • 1
    Pretty sure that the default constructor should do the time initialisation as is done in second one - whereas the latter one should only initialise the objects with the constructor arguments... – Aconcagua Nov 13 '18 at 18:35
1

You've mixed up the ctor code a bit, leaving the member vars uninitialized when you use the default ctor.

Time::Time()
{
    // Initialize to the current time
    time_t currenttime;
    struct tm timeinfo;
    time(&currenttime);
    localtime_s(&timeinfo, &currenttime);
    hour = timeinfo.tm_hour;
    minute = timeinfo.tm_min;
    second = timeinfo.tm_sec;
}

// Modified to use initializer list
Time::Time(int h, int m, int s) :
    hour(h), minute(m), second(s)
{
}
001
  • 13,291
  • 5
  • 35
  • 66