1

Here is the class

    class graph {
    public:
        graph() {}; // constructor
        graph(int size);
        friend ostream& operator<< (ostream& out, graph g);
    private:
        int size;
        bool** grph;
    };

This is how I generate the graph:

    graph::graph(int size) {
        grph = new bool*[size];
        for (int i = 0; i < size; ++i)
            grph[i] = new bool[size];
        for (int i = 0; i < size; ++i)
            for (int j = i; j < size; ++j) {
                if (i == j)
                    grph[i][j] = false;
                else {
                    cout << prob() << endl;//marker
                    grph[i][j] = grph[j][i] = (prob() < 0.19);
                    cout << grph[i][j] << endl;//marker
                }
            }
        cout << "Graph created" << endl;//marker
    }

The constructor and the prob() function work just fine. I have tested them using the markers.

This is where I believe the problem exists. This is the code for the overloaded operator <<

    ostream& operator<< (ostream& out, graph g) {
        for (int i = 0; i < g.size; ++i) {
            for (int j = 0; j < g.size; ++j) 
                out << g.grph[i][j] << "\t";
            out << endl;
        }
        return out;
    }

Here is how this is called.

     graph g(5);
 cout << g << endl;

Now, the program compiles just fine. But, while execution, the graph is not being printed. I haven been able to print the graph the same way without overloading the operator, but by having the for loops run inside the main or by using a class member function.

Can anyone help me out? I am using Visual Studio 2015.

Apara
  • 375
  • 4
  • 14
  • 1
    This class will also need to overload some functions to accommodate copying. This post should be useful: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three – Galik Aug 04 '16 at 19:05

3 Answers3

2

The loops are never entered, because

i < g.size

is always false, because g.size is 0! You are never actually setting the member variable size to the size entered by the user, so it defaults to 0.

You'll have to set it, i.e.

this->size = size; //Sets member variable 'size' to 'size' from the function arguments

Also, you didn't specify a copy constructor, so the implicit one will be used. But the implicit one just copies the values, and so the pointer grph will point in 2 objects to the same data. You're leaking memory, that's why it doesn't matter (technically), but you should implement a proper destructor and copy/move constructor.

But because operator<< should only be printing, consider passing by const& instead of by value!

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
0

The problem depends by the missed initialization of the class attribute size in the costructor method.

Adding to your ctor this.size = size should resolve your problem as already rightly indicated by Rakete1111.

The following method allows you to print false or true, otherwise you may try cout<<std::boolalpha flag

ostream& operator<< (ostream& out, graph g) {
        for (int i = 0; i < g.size; ++i) {
            for (int j = 0; j < g.size; ++j) 
                out << (g.grph[i][j] ? "true" : "false") << "\t";
            out << endl;
        }
        return out;
    }
Ciro Corvino
  • 2,038
  • 5
  • 20
  • 33
  • 1
    While this code snippet may solve the question, [including an explanation](http://meta.stackexchange.com/questions/114762/explaining-entirely-‌​code-based-answers) really helps to improve the quality of your post. Remember that you are answering the question for readers in the future, and those people might not know the reasons for your code suggestion. – NathanOliver Aug 04 '16 at 18:31
  • I saw better your costructor and in effect the cause is the missed initialization of the "size" attribute... – Ciro Corvino Aug 04 '16 at 19:11
0

In addition to what others have said (you are not initializing the size member), your operator<< is accepting the graph object by value. When the operator is called, a copy of the input graph object is made, but your graph class is not properly written to support copying. Your operator<< should accept the graph object by const reference instead so a copy is not needed:

ostream& operator<< (ostream& out, const graph &g)

Your graph class does not support copying because you are completely violating the Rule of Three:

  1. Your default constructor is not initializing the graph data at all.

  2. You are missing a destructor that frees the graph data.

  3. You are missing a copy constructor and copy assignment operator to make a copy of the graph data from one graph object to another.

Even if you pass the graph object to operator<< by reference, you still need to implement the Rule of Three correctly to avoid problems in your code in general.

You need to either:

  1. implement proper constructors and operators:

    class graph {
    public:
        graph(int size = 0);
        graph(const graph &src);
        ~graph();
    
        graph& operator=(const graph &rhs);
    
        friend std::ostream& operator<< (std::ostream& out, const graph &g);
    private:
        int size;
        bool** grph;
    };
    
    std::ostream& operator<< (std::ostream& out, const graph &g);
    

    graph::graph(int size)
        : grph(NULL), size(size)
    {
        grph = new bool*[size];
        for (int i = 0; i < size; ++i) {
            grph[i] = new bool[size];
        }
        for (int i = 0; i < size; ++i) {
            for (int j = i; j < size; ++j) {
                if (i == j) {
                    grph[i][j] = false;
                } else {
                    grph[i][j] = grph[j][i] = (prob() < 0.19);
                }
            }
        }
    }
    
    graph::graph(const graph &src)
        : grph(NULL), size(src.size)
    {
        grph = new bool*[size];
        for (int i = 0; i < size; ++i) {
            grph[i] = new bool[size];
            for (int j = i; j < size; ++j) {
                grph[i][j] = src.grph[i][j];
        }
    }
    
    graph::~graph() {
        for (int i = 0; i < size; ++i) {
            delete[] grph[i];
        }
        delete[] grph;
    }
    
    graph& graph::operator=(const graph &rhs)
    {
        if (this != &rhs)
        {
            graph tmp(rhs);
            std::swap(grph, tmp.grph);
            std::swap(size, tmp.size);
        }
        return *this;
    }
    
    std::ostream& operator<< (std::ostream& out, const graph &g) {
        for (int i = 0; i < g.size; ++i) {
            for (int j = 0; j < g.size; ++j) {
                out << g.grph[i][j] << "\t";
            }
            out << endl;
        }
        return out;
    }
    
  2. change graph to use std::vector instead of allocating the arrays manually. Let the compiler handle all of the memory management and copying for you:

    class graph {
    public:
        graph(int size = 0);
        friend ostream& operator<< (ostream& out, const graph &g);
    private:
        std::vector<std::vector<bool> > grph;
    };
    
    std::ostream& operator<< (std::ostream& out, const graph &g);
    

    graph::graph(int size)
    {
        grph.resize(size);
        for (int i = 0; i < size; ++i) {
            grph[i].resize(size);
        }
        for (int i = 0; i < size; ++i) {
            for (int j = i; j < size; ++j) {
                if (i == j) {
                    grph[i][j] = false;
                } else {
                    grph[i][j] = grph[j][i] = (prob() < 0.19);
                }
            }
        }
    }
    
    ostream& operator<< (ostream& out, const graph &g) {
        for (int i = 0; i < g.grph.size(); ++i) {
            std:::vector<bool> &row = g.grph[i];
            for (int j = 0; j < row.size(); ++j) {
                out << row[j] << "\t";
            }
            out << endl;
        }
        return out;
    }
    
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770