10

In my C++ project, I have two classes, Particle and Contact. In the Particle class, I have a member variable std::vector<Contact> contacts which contains all contacts of a Particle object, and corresponding member functions getContacts() and addContact(Contact cont). Thus, in "Particle.h", I include "Contact.h".

In the Contact class, I would like to add code to the constructor for Contact that will call Particle::addContact(Contact cont), so that contacts is updated for both the Particle objects between which the Contact object is being added. Thus, I would have to include "Particle.h" in "Contact.cpp".

My question is whether or not this is acceptable/good coding practice and, if not, what would be a better way to implement what I am trying to achieve (simply put, automatically updating the list of contacts for a specific particle whenever a new contact is created).


These classes will be tied together by a Network class that will have N particles (std::vector<Particle> particles) and Nc contacts (std::vector<Contact> contacts). But I wanted to be able to have functions like particles[0].getContacts() – is it okay to have such functions in the Particle class in this case, or is there a better association "structure" in C++ for this purpose (of two related classes being used in another class).


I may need a perspective shift here in how I am approaching this. Since the two classes are connected by a Network class object, is it typical code/class organization to have connectivity information entirely controlled by the Network object (in that a Particle object should not be aware of its contacts and, consequently, it should not have a getContacts() member function). Then, in order to know what contacts a specific particle has, I would need to obtain that information through the Network object (e.g., using network.getContacts(Particle particle)).

Would it be less typical (perhaps even discouraged) C++ class design for a Particle object to have that knowledge, as well (i.e., have multiple ways to access that information -- through either the Network object or the Particle object, whichever seems more convenient)?

  • 4
    Here's a talk from cppcon 2017 - The Three Layers of Headers : youtu.be/su9ittf-ozk – Robert Andrzejuk Nov 8 '17 at 12:18
  • 3
    Questions that contain words like "best," "better" and "acceptable" are unanswerable unless you can state your specific criteria of evaluation. – Robert Harvey Nov 8 '17 at 16:30
  • Thanks for the edit, though changing your wording to "typical" just makes it a question of popularity. There are reasons why coding is done one way or another, and while popularity can be an indication that a technique is "good" (for some definition of "good"), it can also be an indication of cargo-culting. – Robert Harvey Nov 8 '17 at 16:37
  • @RobertHarvey I removed "better" and "bad" in my final section. I suppose I am asking for the typical (perhaps even favored/encouraged) approach when you have a Network class object that contains Particle objects and Contact objects. With that base knowledge, I can then try to assess whether or not it fits with my specific needs, which are still being explored/developed as I go along in the project. – AnInquiringMind Nov 8 '17 at 16:38
  • @RobertHarvey I suppose that I am new enough to writing C++ projects completely from scratch that I am fine with learning what is "typical" and "popular". Hopefully I will gain enough insight at some point to be able to realize why another implementation is actually better, but for now, I just want to make sure I am not approaching this in a completely bone-headed way! – AnInquiringMind Nov 8 '17 at 16:40
18

There are two parts in your question.

The first part is the organization of C++ header files and source files. This is solved by using forward declaration and the separation of the class declaration (putting them in the header file) and method body (putting them in the source file). Furthermore, in some cases one can apply the Pimpl idiom ("pointer to implementation") to solve harder cases. Use shared-ownership pointers (shared_ptr), single-ownership pointers (unique_ptr), and non-owning pointers (raw pointer, i.e. the "asterisk") according to best practices.

The second part is how to model objects that are inter-related in the form of a graph. General graphs that are not DAGs (directed acyclic graphs) don't have a natural way of expressing tree-like ownership. Instead, the nodes and connections are all metadata that belong to a single graph object. In this case, it is not possible to model the node-connection relationship as aggregations. Nodes don't "own" connections; connections don't "own" nodes. Instead, they are associations, and both nodes and connections are "owned by" the graph. The graph provides query and manipulation methods that operate on the nodes and connections.

  • Thanks for the response! I actually do have a Network class that will have N particles and Nc contacts. But I wanted to be able to have functions like particles[0].getContacts() – are you suggesting in your last paragraph that I shouldn't have such functions in the Particle class, or that the current structure is okay because they are inherently related/associated through Network? Is there a better association "structure" in C++ in this case? – AnInquiringMind Nov 8 '17 at 12:46
  • 1
    Generally that the Network is responsible for knowing about relationships between objects. If you use an adjacency list, for example, the particle network.particle[p] would have a corresponding network.contacts[p] with the indices of its contacts. Otherwise, the Network and the Particle are somehow both tracking the same information. – Useless Nov 8 '17 at 15:55
  • @Useless Yeah, that's where I'm unsure of how to proceed. So you are saying that the Particle object shouldn't be aware of its contacts (so I shouldn't have a getContacts() member function), and that that information should only come from within the Network object? Would it be bad C++ class design for a Particle object to have that knowledge (i.e., have multiple ways to access that information -- through either the Network object or the Particle object, whichever seems more convenient)? The latter seems to make more sense to me, but maybe I need to change my perspective on this. – AnInquiringMind Nov 8 '17 at 16:09
  • 1
    @PhysicsCodingEnthusiast: The problem with a Particle knowing anything about Contacts or Networks is that it ties you to a specific way of representing that relationship. All three classes may have to agree. If instead the Network is the only one who knows or cares, that's only one class that needs to change if you decide another representation is better. – cHao Nov 8 '17 at 16:39
  • @cHao Okay, this makes sense. So Particle and Contact should be completely separate, and the association between them is defined by the Network object. Just to be completely sure, this is (probably) what @rwong meant when he/she wrote, "both nodes and connections are "owned by" the graph. The graph provides query and manipulation methods that operate on the nodes and connections.", right? – AnInquiringMind Nov 8 '17 at 16:48
5

If I got you right, the same contact object belongs to more than one particle object, since it represents some kind of physical contact between two or more particles, right?

So the first thing which I think is questionable is why Particle has a member variable std::vector<Contact>? It should be a std::vector<Contact*> or a std::vector<std::shared_ptr<Contact> > instead. addContact then should have different signature like addContact(Contact *cont) or addContact(std::shared_ptr<Contact> cont) instead.

This makes it unnecessary to include "Contact.h" in "Particle.h", a forward declaration of class Contact in "Particle.h", and an include of "Contact.h" in "Particle.cpp" will be enough.

Then the question about the constructor. You want something like

 Contact(Particle &p1, Particle &p2)
 {
      p1.addContact(this);
      p2.addContact(this);
 }

Right? This design is ok, as long as your program does always knows the related particles at the point in time when a contact object has to be created.

Note, if you go the std::vector<Contact*> route, you have to invest some thoughts about the lifetime and ownership of the Contact objects. No particle "owns" its contacts, a contact will probably have to be deleted only if both related Particle objects are destructed. Using std::shared_ptr<Contact> instead will solve this problem for you automatically. Or you let a "surrounding context" object take the ownership of particles and contacts (like suggested by @rwong), and manage their lifetime.

  • I don't see the benefit of addContact(const std::shared_ptr<Contact> &cont) over addContact(std::shared_ptr<Contact> cont)? – Caleth Nov 8 '17 at 9:28
  • @Caleth: this was discussed here: stackoverflow.com/questions/3310737/… - "const" is not really important here, but passing objects by reference (and scalars by value) is the standard idiom in C++ . – Doc Brown Nov 8 '17 at 10:05
  • 2
    Many of those answers seem to be from a pre-move paradigm – Caleth Nov 8 '17 at 10:16
  • @Caleth: ok, to keep all nitpickers happy, I changed this quite unimportant part of my answer. – Doc Brown Nov 8 '17 at 10:52
  • 1
    @PhysicsCodingEnthusiast: no, this is foremost about making particle1.getContacts() and particle2.getContacts() delivering the same Contact object representing the physical contact between particle1 and particle2, and not two different objects. Of course, one could try to design the system in a way it does not matter if there are two Contact objects available at the same time representing the same physical contact. This would involve to make Contact immutable, but are you sure that this is what you want? – Doc Brown Nov 8 '17 at 13:38
0

Yes, what you describe is a very acceptable way of ensuring that every Contact instance is in the list of contacts of a Particle.

  • Thanks for the response. I had read some suggestions that having a pair of interdependent classes is to be avoided (for instance, in "C++ Design Patterns and Derivatives Pricing" by M. S. Joshi), but apparently that is not necessarily correct? Out of curiosity, is there perhaps another way of implementing this automatic updating without needing interdependence? – AnInquiringMind Nov 8 '17 at 8:05
  • 4
    @PhysicsCodingEnthusiast: Having interdependent classes creates all kinds of difficulties and you should try to avoid them. But sometimes, two classes are so closely related to each other that removing interdependence between them causes more problems than the interdependence itself. – Bart van Ingen Schenau Nov 8 '17 at 9:01
0

What you have done is correct.

Another way... If the aim is to ensure that every Contact is in a list, then you could:

  • block creation of Contact (private constructors),
  • forward declare Particle class,
  • make Particle class a friend of Contact,
  • in Particle create a factory method which creates a Contact

Then you don't have to include particle.h in contact

  • Thanks for the response! That seems like a useful way of implementing this. Just wondering, with my edit to the initial question regarding the Network class, does that change the suggested structure, or would it still be the same? – AnInquiringMind Nov 8 '17 at 13:07
  • After You have updated your question, it is changing scope. ... Now you are asking about the architecture of your application, when previously it was about a technical issue. – Robert Andrzejuk Nov 8 '17 at 19:34
0

Another option you might consider is making the Contact constructor that accepts a Particle reference templated. This will allow a Contact to add itself to any container that implements addContact(Contact).

template<class Container>
Contact(/*parameters*/, Container& container)
{
  container.addContact(*this);
}

protected by gnat Nov 9 '17 at 15:50

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.