Search code examples
c++exceptionout-of-memorybreadth-first-searchbad-alloc

bad alloc exception when trying to resolve BFS challenge on HackerRank


So i was trying to make the challage: Breadth First Search: Shortest Reach on HackerRank, but i keep getting the bad alloc exception when the tests have great numbers of node/edges. The program works on the first test, so i don't think, it's something wrong with the implementation. So here is the implementation: (sorry for the indentation , my first question)

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
#include <queue>
#include <limits.h>
using namespace std;


int main() {
//test numbers
int t;
//the cost
int cost = 6;
cin >> t;
//for each test
for (int nt = 0; nt < t; ++nt) {
    int n, e;
    int snode;
    queue <int> *que = new queue<int>();
    //read the node/edges
    cin >> n >> e;
    //distance/visited/parents arrays/adjlist vector
    int dist[n + 1] = {-1};
    bool visited[n + 1] = {false};
    int parents[n + 1] = {-1};
    vector< vector<int> > adjList(n + 1);

    //read into the adjlist, unoriented graph, each edge has 6 weight
    for (int ne = 0; ne < e; ++ne) {
        int x, y;
        cin >> x >> y;
        adjList[x].push_back(y);
        adjList[y].push_back(x);
    }

    //read the starting node
    cin >> snode;
    dist[snode] = 0;

    //do actual bfs
    que->push(snode);
    visited[snode] = true;
    while(!que->empty()) {
        int c_node = que->front();
        que->pop();
        for (int i = 0; i < adjList[c_node].size(); ++i) {
            if (visited[adjList[c_node].at(i)] == false) {
                que->push(adjList[c_node].at(i));
                parents[adjList[c_node].at(i)] = c_node;
                dist[adjList[c_node].at(i)] = dist[parents[adjList[c_node].at(i)]] + cost;
                visited[adjList[c_node].at(i)] == true;
            }
        }
    }

    //print at output the distance from the starting node to each other node
    //if unreachable, print -1
    for (int i = 1; i < n + 1; ++i) {
        if (i == snode) {

        } else if (dist[i] == 0 && i != snode) {
            cout << "-1 ";
        } else {
            cout << dist[i] << " ";
        }
    }
    cout << "\n";
}
return 0;
}

Am i doing something wrong, i haven't seen anyone else complain on this matter in the discussion section of the site. How can i avoid the exception to be thrown and from where does it come? Thank you!


Solution

  • I don't know, exactly, what is the cause of your exception; and I don't know ho to reproduce your problem because depends (I suppose) from the input values. A lot of input values, I suppose.

    But I see some weak points (IMHO) of your code, so I try to point your attention to them.

    1) you alloc a std::queue in your for cycle

    queue <int> *que = new queue<int>();
    

    but you never free it; it's a waste of memory

    2) you're using C-style variable-length arrays

    int dist[n + 1] = {-1};
    bool visited[n + 1] = {false};
    int parents[n + 1] = {-1};
    

    They aren't valid C++ standard code. I suggest you the use of standard containers (std::vector or std::queue).

    3) you're initializing your C-style variable-length arrays with a initializers lists with only an element (-1 or false). I suppose your intention was initialize all n+1 elements with -1 and false. But this syntax initialize only the first element of the array with -1 and false.

    If you want to initialize all n+1 element to -1 and false, the solution is (again) use standard containers; by example

    std::vector<int>   dist(n+1, -1);
    std::vector<bool>  visited(n+1, false);
    std::vector<int>   parents(n+1, -1);
    

    4) you access arrays without bounds checking. By example:

    cin >> snode;
    dist[snode] = 0;
    

    where snode is a int variable; if you insert a negative value, or a value over n, you write dist out of its bounds, devastating the memory. This, I suppose, can explain your "bad alloc exception".

    Suggestion: use standard containers (again) instead of C-style array and use at() (that perform bounds checking) instead []; so

    cin >> snode;
    dist.at(snode) = 0;
    

    5) sorry for my bad English (ok, I'm joking: this isn't one of your weak points; this is one of mine).