Private class members not fully encapsulated?

Question

This is a follow up post from Instance-level encapsulation with C++.

I've defined a class and created two objects from that class.

#include <iostream>
#include <ctime>
#include <string>

using namespace std;

class timeclass {
  private:
  string date;

  time_t gmrawtime, rawtime;
  struct tm * timeinfo;
  char file_date[9];

  void tm_init(int);

public:
  timeclass(int);
  void print_date();
};

void timeclass::tm_init(int y) {
  timeinfo = gmtime(&rawtime);
  timeinfo->tm_year = y - 1900; // timeinfo->tm_year holds number of years since 1900
  timeinfo->tm_mon = 0;
  timeinfo->tm_mday = 1;
  timeinfo->tm_hour = 0;
  timeinfo->tm_min= 0;
  timeinfo->tm_sec= 0;
}

timeclass::timeclass(int y) {
  timeclass::tm_init(y);
  gmrawtime = mktime(timeinfo) - timezone; 
}

void timeclass::print_date() {
  strftime(file_date,9,"%Y%m%d",timeinfo);

  date = string(file_date);
  cout<<date<<endl;
}

/* -----------------------------------------------------------------------*/

int main()
{
  timeclass time1(1991); 
  timeclass time2(1992); 

  time1.print_date(); // Prints 19920101, despite being initialized with 1991
  time2.print_date(); // Prints 19920101, as expected

  return 0;
}

This example is part of a date counter sliced and diced from my main program, but it illustrates my point. I want to have a date counter running for each instance of the class (time1 and time2), but it looks like once I construct the time2 object, the 'timeinfo' variable that I thought was encapsulated in time1 gets overwritten by the time2 constructor.

I am aware that C++ supports only class-level encapsulation, and am wondering if my problem is because members of the same class have access to one another's private members. Is there a way around this, so I can achieve what I want to do? Thank you, Taylor

Avoid doing `using namespace std;`. See [here](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-a-bad-practice-in-c) for an explanation. — AxelOmega, Mar 08 '13 at 21:21
Thanks @AxelOmega, I welcome any hints since I'm no expert on this stuff. Would you suggest omitting `using namespace std;` altogether and then simply calling std::cout (and other functions other than cout) explicitly? — Taylor, Mar 08 '13 at 21:31
Yes `std::cout` is normal. It is what is normal in most C++ code also. You can also do `using std::cout` if you feel you can not type five characters more. But `std::` becomes a reflex after a while. — AxelOmega, Mar 08 '13 at 21:34

Shafik Yaghmour · Answer 1 · 2013-03-09T01:37:47.277

5

gmtime(), localtime(), ctime() and asctime() return a pointer to static data. So subsequent calls may overwrite information written by previous calls. This also means that these calls are not thread-safe although in this case multiple threads are not involved.

Other answers have provided possible workarounds for this limitation.

edited Mar 09 '13 at 01:37

answered Mar 08 '13 at 21:16

Shafik Yaghmour

154,301
39
440
740

score 2 · Answer 2 · edited May 23 '17 at 11:57

2

You don't actually want gmtime() (see Shafik's answer). You just want a std::tm you can modify:

void timeclass::tm_init(int y) {
  timeinfo = new std::tm;
  timeinfo->tm_year = y - 1900;
  timeinfo->tm_mon = 0;
  timeinfo->tm_mday = 1;
  timeinfo->tm_hour = 0;
  timeinfo->tm_min= 0;
  timeinfo->tm_sec= 0;
}

As Shafik already wrote, your problem is the internal static std::tm used by many *time() methods which you point to. So just create your own std::tm, or even simplier, use it as a member instead of your pointer:

class timeclass {
  private:
  std::tm timeinfo;
  /* rest stays the same */
};

void timeclass::tm_init(int y) {
  timeinfo = *std::gmtime(&rawtime); // if you need gmtime
  timeinfo.tm_year = y - 1900;
  timeinfo.tm_mon = 0;
  timeinfo.tm_mday = 1;
  timeinfo.tm_hour = 0;
  timeinfo.tm_min= 0;
  timeinfo.tm_sec= 0;
}

edited May 23 '17 at 11:57

Community

1
1

answered Mar 08 '13 at 21:19

Zeta

103,620
13
194
236

Thanks @zeta, I'll play with that for a bit, now my code is heavily dependent on `gmtime()` so I'll try to make it work. – Taylor Mar 08 '13 at 21:26
@Taylor: If you really need `gmtime()` you can still use the `std::tm` approach, since `std::tm` should be POD: `timeinfo = *gmtime(&rawtime)`. – Zeta Mar 08 '13 at 21:51
great, I'm sure it could be make to work without `gmtime()` but this is the simplest solution. I've thrown a few `&`'s around since timeinfo is no longer a pointer, and it seems to run. Somewhere I've lots the ability to increment my date (not included in my example above), but I'll work on that tomorrow and get back. Thanks again. – Taylor Mar 08 '13 at 22:08

Peter Wood · Answer 3 · 2013-03-08T22:20:23.737

0

As others have pointed out, the problem is that the functions you are using return global data. So your question has been side-stepped.

However, as you point out, C++ encapsulates at the class level rather than object level, so any object can modify private data of any other object of the same class.

You can get round this by only using abstract classes as parameters and class members:

class Time {
public:
    virtual void setYear(int year) = 0;
    virtual void printDate() = 0;
    virtual void subtract(Time& otherTime) = 0;   
};

edited Mar 08 '13 at 22:20

answered Mar 08 '13 at 21:35

Peter Wood

23,859
5
60
99

They return global data which causes the problem, but thread safety has nothing to do with it (could be a thread-local global, but the problem would still occur). – GManNickG Mar 08 '13 at 22:04

Private class members not fully encapsulated?

3 Answers3

Linked