Using or not using new for creation of a class in another

Question

I have been solving a question, Dijkstra's Algorithm, in C++. I've implemented it using adjacency list.

So I have a class for a node, a class for a minHeap, and a class for the Graph.

class node
{
    int vertex,weight;
    node *next;
    friend class Graph;
    friend class minHeap;
public:
    node();
    node(int,int);
};
node::node(){
    vertex=weight=0;
    next=0;
}
node::node(int v,int wt){
    vertex=v;
    weight=wt;
    next=0;
}

Do I define the minHeap class this way (without a friend function) and create an object in the getDijkSP() function normally, which allows me to use the object only in that function?

class minHeap
{
    node *heap;
    int heapSize,capacity,*pos;
public:
    minHeap(int);
    void addElement(node);
    node extractMin();
    void minHeapify(int);
    void decreaseKey(int,int);
};
minHeap::minHeap(int cap){
    heap=new node[capacity=cap];
    heapSize=-1;
    pos=new int[cap]();
}                                        //eliminating other methods

class Graph
{
    node **adjList;
    int v;
    bool *visited;
public:
    Graph(int);
    void addEdge(int,int,int);
    void removeEdge(int,int);
    bool existsEdge(int,int);
    void getDijkSP();
};
Graph::Graph(int vertices){
    adjList=new node*[v=vertices];
    for(int i=0;i<v;i++)
        adjList[i]=NULL;
}
void Graph::getDijkSP(){
    minHeap hp(v);                            //here
    hp.addElement(node(0,0));
    for(int i=1;i<v;i++)
        hp.addElement(node(i,INT_MAX));
    while(!hp.isempty()){
        node temp=hp.extractMin();
        cout<<temp.vertex<<" "<<temp.weight<<endl;
        for(node *current=adjList[temp.vertex];current;current=current->next)
            hp.decreaseKey(current->vertex,current->weight+temp.weight);
    }
}

(OR) Do I define the minHeap class with a friend function, so that I can create an object of the minHeap class using the new keyword? (And this helps me define the minHeap object in the scope of the Graph class, so that I can use it in all of its functions for other capabilities as well.)

class minHeap
{
    node *heap;
    int heapSize,capacity,*pos;
    friend class Graph;                //say like this
public:
    minHeap(int);
    void addElement(node);
    node extractMin();
    void minHeapify(int);
    void decreaseKey(int,int);
};
minHeap::minHeap(int cap){
    heap=new node[capacity=cap]();
    heapSize=-1;
    pos=new int[cap]();
}

class Graph
{
    node **adjList;
    int v;
    bool *visited;
    minHeap *hp;                                //and do this
public:
    Graph(int);
    void addEdge(int,int,int);
    void removeEdge(int,int);
    bool existsEdge(int,int);
    void getDijkSP();
};
Graph::Graph(int vertices){
    adjList=new node*[v=vertices];
    for(int i=0;i<v;i++)
        adjList[i]=NULL;
    hp=new minHeap(v);                        //dynamic allocation
}
void Graph::getDijkSP(){
    hp->addElement(node(0,0));
    for(int i=1;i<v;i++)
        hp->addElement(node(i,INT_MAX));
    while(!hp->isempty()){
        node temp=hp->extractMin();
        cout<<temp.vertex<<" "<<temp.weight<<endl;
        for(node *current=adjList[temp.vertex];current;current=current->next)
            hp->decreaseKey(current->vertex,current->weight+temp.weight);
    }
}

I have read this and a few other articles, but specifically want to know the advantages, disadvantages and the appropriateness of both the methods for such similar kinds of questions.

I've provided the constructors for the classes for better clarity.

Answer 1

Short answer would be NO. I would suggest you to read up on smart pointers and rewrite this whole mess. In C++ there is no real reason to use manual allocation in so simple project as this ever.

Also instead of assigning 0 or NULL to a pointer use nullptr, which is C++ symbol only for null pointers unlike the previous mentioned C values that are actually just a int 0 which may cause some unintentional errors.

Edit in response to your comment:

So I've decided to rewrite your code using actual modern C++ instead of this C code with simple classes. In your whole example there are almost no pointers or dynamic allocations needed. I wasn't absolutely sure who exactly should own the actual nodes so from the example I assumed that the MinHeap should. Also I didn't get the point of MinHeap::pos and Graph::visited from what I could see. I can explain any part of that code in more detail, just ask which.

Here is the code:

class Node {
    // Only friend class required if you insist on keeping members of Node private.
    // If they aren't meant to change, consider declaring them as public and const.
    template <unsigned Size> friend class Graph;
public:
    Node(int v, int wt) : vertex(v), weight(wt) {}

private:
    // Default values written in here right after declarations
    // There is no need for a default constructor. You never call it anyway.
    int vertex;
    int weight;
    Node* next = nullptr;
};

// Template parameter because of internal use of std::array.
// If the capacity shouldn't be constant, use std::vector and remove template.
template <unsigned Capacity>
class MinHeap {
public:
    // No constructor needed
    // ---------------------

    // One small tip: write parameter names in function declarations
    // even if they aren't needed there for better readability of your code.
    void addElement(Node n) { /* impl */ }
    Node extractMin() { /* impl */ }

    unsigned capacity() { return Capacity; }
    bool isEmpty() { return heap.isEmpty(); }

private:
    // Default values written in here right after declarations
    int heapSize = -1;
    std::array<Node, Capacity> heap;
};

// Template parameter because of internal use of std::array.
// If the vertex count shouldn't be constant, use std::vector and remove template.
template <unsigned Vertices>
class Graph {
public:
    // No constructor needed
    // ---------------------

    void getDjikSP() {
        hp.addElement({0, 0});
        for (unsigned i = 1; i < hp.capacity(); ++i)
            hp.addElement({0, INT_MAX});

        while (!hp.isEmpty()) {
            Node tmp = hp.extractMin();
            std::cout << tmp.vertex << " " << tmp.weight << std::endl;

            for (Node* current = adjList[tmp.vertex]; current != nullptr; current = current->next)
                hp.decreaseKey(current->vertex, current->weight + tmp.weight);
        }
    }

private:
    // Default values written in here right after declarations
    std::array<Node*, Vertices> adjList;
    MinHeap<Vertices> hp;
};

There is still a lot of space for improvements of this code, for example the MinHeaP::extractMin should maybe return Node&& if it is removed from the heap or const Node& if it should return a reference to the top, etc. To address all the problems and inefficiencies this can still have I would need to see the full code with all functions.