-3

My question is explained in the example below. It is weird that the parent object loses the internal data of its child object, if the parent object is instantiated before the child object populates its internal data.

The code has a TicketService, which internally holds a TicketStore instance. The store is populated with 20 concerts. If the code has the following order, everything works as expected, and the service will be able to see 20 concerts in the store.

TicketStore store;
Add20Concerts(store);
TicketService service(store);   // <- instantiating service after populating the store
// # of concerts in service=20
cout<<"# of concerts in service="<<service.GetNumberOfConcerts()<<endl;

However, if service is instantiated before the store is populated, it sees zero concert in the store.

TicketStore store;
TicketService service(store);   // <- instantiating service before populating the store
Add20Concerts(store);
// # of concerts in service=20
cout<<"# of concerts in service="<<service.GetNumberOfConcerts()<<endl;

Why service has a store that is empty? If service is instantiated by passing a reference to store, and since store will use a default copy constructor, the underlying concert_map_ should have already held the 20 concerts.

A more-detailed skeleton of the code is as below, but the above is pretty much what my confusion is.

*******************************************************************************/
// ticket_service.h
class TicketService {
 public:

  explicit TicketService(const TicketStore& store) {
    store_ = store;  // should use the default copy constructor, which is a shallow copy of the original store
  }

  int32_t GetNumberOfConcerts() {
    return store_.GetStoreSize();
  }

 private:
  TicketStore store_;
};

*******************************************************************************/
// ticket_store.h
class TicketStore {
 public:
  TicketStore() {}

  void AddConcert(const Concert& concert) {
    concert_map_[concert.id()] = concert;
  }

  int32 GetStoreSize() const {
    return concert_map_.size();
  }

 private:
  absl::flat_hash_map<int64, Concert> concert_map_;
};

*******************************************************************************/
// main.cc
int main(int argc, char* argv[]) {
  TicketStore store;
  TicketService service(store);
  Add20Concerts(store);
  
  // if service is instantiated here, # of concerts in service=20.
  // TicketService service(store);
  
  cout<<"store size="<<store.GetStoreSize()<<endl; // store size=20
  
  // # of concerts in service=0 ??
  cout<<"# of concerts in service="<<service.GetNumberOfConcerts()<<endl;
}

void Add20Concerts(TicketStore &store) {
  for(int i=0; i<10; i++) {
    Concert concert;
    concert.set_id(i);
    store.AddConcert(concert);
  }
}
Robert
  • 65
  • 1
  • 7
  • I can't be certain without a [mre] but it looks like you have a [object slicing issue](https://stackoverflow.com/questions/274626/what-is-object-slicing) – NathanOliver Jan 04 '21 at 22:10
  • @NathanOliver There is no object slicing occurring in this code – Remy Lebeau Jan 04 '21 at 22:11
  • 3
    `class TicketService: public TicketService` -- I'll be surprised if any self-respecting C++ compiler will accept this. Please show real C++ code that meets all requirements of a [mre], as explained in the [help]. For more information see [ask] questions in the [help]. Your question cannot be authoritatively answered without showing valid code that meets all requirements of a [mre]. – Sam Varshavchik Jan 04 '21 at 22:11
  • 1
    `void Add20Concerts(store)` is also wrong and should not compile. Is the `store` being passed in **by value** or **by reference**? – Remy Lebeau Jan 04 '21 at 22:16
  • My apology for the mistake in the more-detailed part. It was meant to provide a context but unfortunately some additional confusion was introduced. I have updated the code to avoid this confusion, and I appreciate @RemyLebeau jumped over the bad part and provided an answer to my question. – Robert Jan 05 '21 at 06:27

1 Answers1

2

The TicketService constructor is making a copy of the TicketStore object that is passed in to it. In your second example, that object is empty when the service object is created. Any changes made to the store object in main() after the service object is created will not be reflected in that copy.

For what you are attempting, the TicketService object would need to hold a reference to the TicketStore object, eg:

class TicketService {
 public:
  explicit TicketService(const TicketStore& store) : store_(store) {}

  ...

 private:
  TicketStore& store_; // <-- reference!
};

UPDATE: to avoid any possibility of a dangling reference, a better option would be to use a std::shared_ptr instead, eg:

class TicketService {
 public:
  explicit TicketService(std::shared_ptr<TicketStore> store) : store_(store) {}

 int32_t GetNumberOfConcerts() {
   return (store_) ? store_->GetStoreSize() : 0;
 }

 private:
  std::shared_ptr<TicketStore> store_;
};
void Add20Concerts(TicketStore &store) {
  for(int i=0; i<10; i++) {
    Concert concert;
    concert.set_id(i);
    store.AddConcert(concert);
  }
}

int main() {
  auto store = std::make_shared<TicketStore>();
  TicketService service(store);
  Add20Concerts(*store);
  
  cout<<"store size="<<store->GetStoreSize()<<endl;  
  cout<<"# of concerts in service="<<service->GetNumberOfConcerts()<<endl;
}

This way, the TicketStore object is guaranteed not to be freed until the TicketService and all shared_ptrs are done using it.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Given the "`class TicketService: public TicketService`" gem in the question, any attempted answer here is pure speculation, and only underscores the futility of attempting to answer low quality questions. – Sam Varshavchik Jan 04 '21 at 22:13
  • 3
    @SamVarshavchik just because the code has a few mistakes in it doesn't mean the *intent* of the code can't be addressed. – Remy Lebeau Jan 04 '21 at 22:19
  • 2
    Storing a reference to an object in the private part of another object like this is a recipe for dangling reference errors. The user of the class needs to know and understand the private internals of the class to use it properly, which is a violation of encapsulation and a sure source of errors down the line. – Chris Dodd Jan 05 '21 at 00:36
  • 1
    @ChrisDodd agreed. I have updated my answer to include an example using `std::shared_ptr` instead – Remy Lebeau Jan 05 '21 at 00:43