0

I am trying use a friend function. The function should be a friend to all the classes that i have. But i get multiple errors some of which says incomplete type. The following are files that i have:

main.cpp

#include <iostream>
#include "my_ClassA.h"
#include "my_ClassB.h"
#include "my_ClassC.h"
#include "my_ClassD.h"


int main()
{
    std::cout<<"Hello World";
    my_namespace::my_ClassA object1;//this will do some computation and set everything up for object1
    my_namespace::my_ClassB object2(object1);//this object2 will use object1 to further do other computation
    my_namespace::my_ClassC object3(object2);
    
    my_namespace::my_classD object4(object4);    
    //runComputation(object1, object2, object3, object4);

    return 0;
}

my_ClassA.h

#pragma once 
#include<string>
#include <vector>

//these three includes are for friend function BUT result in error incomplete type
 #include "my_ClassB.h"
 #include "my_ClassC.h"
 #include "my_ClassD.h"
/////////////////////////////////////////////
namespace my_namespace{
    class my_ClassA{
        friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
            
        
        private:
           std::string name;  
           std::vector<std::string> vec;
          public:
            std::vector<std::string> get_vec();
    };
    void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
    
}

my_ClassA.cpp

#include "my_ClassA.h"

std::vector<std::string> my_namespace::my_ClassA::get_vec(){
    return vec;
}

my_ClassB.h

#pragma once 
#include<string>
#include <vector>

#include "my_ClassA.h"
#include "my_ClassC.h"
#include "my_ClassD.h"
namespace my_namespace{
    class my_ClassB{
        friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
        private:
           std::string name;  
           std::vector<std::string> vec;
          public:
            std::vector<std::string> get_vec();
            my_ClassB(my_ClassA);
            my_ClassB(){
                ;
            }
    };
}

my_ClassB.cpp

#include "my_ClassB.h"

std::vector<std::string> my_namespace::my_ClassB::get_vec(){
    return vec;
}
my_namespace::my_ClassB::my_ClassB(my_ClassA temp_objA){
    ;
}

my_ClassC.h

#pragma once 
#include<string>
#include <vector>
#include "my_ClassB.h"
#include "my_ClassA.h"
 #include "my_ClassD.h"
namespace my_namespace{
    class my_ClassC{
        friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
        private:
           std::string name;  
           std::vector<std::string> vec;
           my_ClassB objB;
          public:
            my_ClassC(my_ClassB);
            std::vector<std::string> get_vec();
    };
}

my_ClassC.cpp

#include "my_ClassC.h"
#include "my_ClassB.h"
std::vector<std::string> my_namespace::my_ClassC::get_vec(){
    return vec;
}
my_namespace::my_ClassC::my_ClassC(my_ClassB temp_objB){
    ;
}

my_ClassD.h

#pragma once 
#include<string>
#include <vector>
#include "my_ClassA.h"
#include "my_ClassB.h"
 #include "my_ClassC.h"

namespace my_namespace{
    class my_ClassD{
        friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
        private:
           std::string name;  
           std::vector<std::string> vec;
           my_ClassA objA;
          public:
            std::vector<std::string> get_vec();
    };
}

my_ClassD.cpp

#include "my_ClassD.h"

std::vector<std::string> my_namespace::my_ClassD::get_vec(){
    return vec;
}

I tried using the getters of each of the classes in the main.cpp. But some my classes have large size vectors,sets etc etc. So i do not want to copy them again and again. So instead i want to access the data members of the classes directly in a function called runComputation. And that function will be taking the object created as references so that copy doesn't happen inside main.cpp.

What i have is this: First i create different objects which may take the previously created object as input in main.cpp. After all the objects are created successfully, i want to run some computation on those objects. Now the problem is that i can use the getters and setters of the objects created in the main.cpp file. But the objects have large vectors and other objects inside them, and so they will be copied each time i use them in a for loop using getters. To avoid this i want to create a friend function that can take these objects as references and avoid copying. How can i resolve this ? And is there any better way of achieving this?

PS: I am aware of ADL. Will ADL be used when i write a friend declaration like friend void runComputation(someobject&); and then after the class to make this friend function visible void runComputation(someobject&);

Jason
  • 36,170
  • 5
  • 26
  • 60
  • Not enough time for a proper answer but the problem is probably mutual inclusion (e.g. A.h includes B.h and vice versa). Simply go through the result of the include directives by hand. You solve this by having only such uses of one class, say A, in B.h for which a "forward declaration" of the fashion `class A;` is sufficient, no header. This is the case if no code in B.h accesses members of A or creates objects but uses only pointers or references, e.g. as function arguments or members (because code for pointers and references can be generated without knowing the implementation of a class). – Peter - Reinstate Monica Jun 08 '21 at 07:06
  • 3
    `I tried using the getters of each of the classes in the main.cpp. But some my classes have large size vectors,sets etc etc. So i do not want to copy them again and again.` just use getters, that returns const reference or non-const referenct – user2807083 Jun 08 '21 at 07:07
  • @George i stated one of the error in my 2nd line. Check it. Now i have highlighted it. Before it was normal text. – Jason Jun 08 '21 at 07:11
  • 1
    The friendship is completely irrelevant - you have a circular dependency between your classes, and you need to break that dependency. Read about forward declarations (and about references) in your favourite C++ book. – molbdnilo Jun 08 '21 at 07:12
  • @molbdnilo yes i think it is mostly because of the circular dependency. I already know about forward declarations and references. Btw what i am not sure about is that can we use getters that return a reference to a data member. For example, normally when i use a ordinary function then returning a reference to a local variable is UB. So does returning a reference to a data member from a getter is okay or is it a bad practice? – Jason Jun 08 '21 at 07:22
  • I don't think you should be using a "friend" at all in this case. Unless they are tightly coupled (which can be a design issue), your don't want the classes to know of each other's existence. The fact that you need to befriend a function, means you want access to class internals. That's also code smell. You break encapsulation. You probably need to define proper interface methods/functions. I.e. you need to rethink the architecture. – JHBonarius Jun 08 '21 at 07:27
  • @JasonLiam The implication was that you should *use* forward references and study some more about references (since you obviously don't know much about them), not that you should be aware of their existence. – molbdnilo Jun 08 '21 at 07:29
  • I know plenty about forward declaration and how to use them. I resoved my problems using them just now. I tried the same thing before posting this question. But that time i was also including the unnecessary headers that resulted in circular dependency. This time i just used forward declarations without including the headers and the program is working. @molbdnilo – Jason Jun 08 '21 at 07:53

4 Answers4

1

Make a my_forwards.h header file. It contains

namespace my_namespace { class my_ClassA; class my_ClassB; class my_ClassC; class my_ClassD; } (or just write this manually at the top of every header file).

Now don't include my_ClassA.h in the other headers at all. If you have the body of a function in my_ClassB that requires the definition of my_ClassA, put it in a .cpp file instead of in the header.

// my_forwards.h
namespace my_namespace {
  class my_ClassA;
  class my_ClassB;
  class my_ClassC;
  class my_ClassD;
  void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
}

// my_ClassA.h
#pragma once
#include<string>
#include <vector>
#include "my_forwards.h"
namespace my_namespace{
  class my_ClassA{
    friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
    
    private:
      std::string name;  
      std::vector<std::string> vec;
    public:
      std::vector<std::string> get_vec();
    };
}

// my_ClassA.cpp
#include "my_ClassA.h"
// include other my_ClassX here if needed, after my_ClassA.h

// implementation of my_ClassA:
std::vector<std::string> my_namespace::my_Class::get_vec() {
  return vec;
}

In some cases this may require declaring a destructor/constructor in a header file, and doing a my_ClassA::my_ClassA()=default; in the cpp file.

Now, your my_ClassC has a member variable of type my_ClassB. This is a case where you have to #include "my_ClassB.h" in the my_ClassC.h header file, because my_ClassC needs the definition of my_ClassB. But in most cases, either a forward declaration is good enough, or slight modification (changing a value parameter to a reference one, for example) is good enough that you don't have to cross-include the header files.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Yes i have done almost the same thing except that i have not created an extra file(header) as you suggested. Instead i have the forward declarations wherever needed. And also the headers wherever needed. The program is working as expected. I have also removed unnecessary forward declarations and circular dependencies. – Jason Jun 08 '21 at 15:24
0

Don't use return by value to avoid copying large vectors, return const reference to access data or non-const reference to modify.

E.g., header:

#pragma once 
#include<string>
#include <vector>

//these three includes are for friend function BUT result in error incomplete type
#include "my_ClassB.h"
#include "my_ClassC.h"
#include "my_ClassD.h"
/////////////////////////////////////////////
namespace my_namespace{
    class my_ClassA{
        friend void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
            
        
        private:
        std::string name;  
        std::vector<std::string> vec;
        public:
            // use this getter to modify inner data by reference 
            std::vector<std::string>& get_vec();

            // use this getter to access data by const referenct
            std::vector<std::string> const & get_vec() const;

    };
    void runComputation(my_ClassA&,my_ClassB&,my_ClassC&,my_ClassD&);
    
}

implementation:

#include "my_ClassA.h"

std::vector<std::string>& my_namespace::my_ClassA::get_vec() {
    return vec;
}


std::vector<std::string> const & my_namespace::my_ClassA::get_vec() const {
    return vec;
}
user2807083
  • 2,962
  • 4
  • 29
  • 37
  • Fair point. It does reduce encapsulation a bit (maybe only exposing a const ref would be best) but that's worth the copy-tradeoff I think. However this doesn't really fix the op's problem. – George Jun 08 '21 at 07:19
0

You may want to refactor classes to a chain of function calls:

// class or struct which contains intermediate state
State state;
runComputation1(&state);
runComputation2(&state);
runComputation3(&state);
runComputation4(&state);
user2807083
  • 2,962
  • 4
  • 29
  • 37
warchantua
  • 1,154
  • 1
  • 10
  • 24
  • This is working approximation, but in this case there is a danger of falling into the "God object" anti-pattern as one of `State` implementation is to collect data of all classes into one object – user2807083 Jun 08 '21 at 07:24
0

The problem is due to circular dependency of different files. Instead of including the headers just use forward declarations for different class parameters and the issue is resolved.

Jason
  • 36,170
  • 5
  • 26
  • 60