Program hanging while running, compiles well

Question

EDITED :

     #include<iostream>
using namespace std;
#include<conio.h>
#include<string.h>

void dfsvisit(int a[][30], int i, const char *color[])
     {
        int v;
        int p[30];
        int f[30]={};
        static int j=1;
        color[i]="gray";

        for(v=1;v<31;v++)
            while(a[i][v]!=0)
                if(!strcmp(color[a[i][v]],"white"))
                    {
                        p[a[i][v]]=i;
                        dfsvisit(a,i,color);
                    }

        color[i]="black";
        j++;
        f[i]=1;
        cout<<f[i]<<" ";
     }

int main()
{
    int a[][30]={{2, 16},{4, 8},{5, 16, 21},{1, 2},{1, 9, 27},{1, 10, 15},{4, 6, 11, 12},{4, 16},{2, 19, 29},{3, 14, 28},{3, 13, 15, 17},{8, 9, 18, 26},{1, 7, 10, 19},{5, 8, 20, 24},{3, 21, 29},{1, 2, 8},{16, 23, 28, 29},{4, 12, 21, 24},   {3, 15, 16},{2, 3, 6, 22},{4, 15, 25},{4, 6, 20, 24},{9, 10, 11, 12},{19, 26, 30},{2, 27, 29},{1, 28, 29, 30},{8, 16, 29},{6, 10, 30},{19, 21, 27},{1, 2, 3, 22}};
    int i;
    const char *color[30];

    for(i=1;i<31;i++)
        color[i]="white";

    for(i=1;i<31;i++)
        if(!strcmp(color[i],"white"))
            dfsvisit(a,i,color);

    return 0;

}

Just want to make sure if I am using const char c correct? If yes, where else have I done the mistake? I am trying to run it on codeblocks. It compiles well, but when I try to run it, it shows program has stopped working.

Thanks.

Answer 1

Change:

for(i=1;i<=30;u++)

to:

for (i = 0; i < 30; i++)

Answer 2

You're comparing pointers to constant strings, which may or may not be equal when the strings are equal. Use an enum for this kind of algorithm:

enum { BLACK, GRAY, WHITE };

Answer 3

You say that this:

for(i=1;i<=30;u++)

compiles?

I suggest changing the u to an i...

EDIT: OK, so you say the u was a typo. Next step: start the thing in a debugger, wait for it to loop, break into it and step through. That should tell you where something happens to your loop variable.

EDIT 2:
< niggle >
I second everybody who suggests you use std::string. You might consider dropping the irregular arrays in favor of std::vectors of std::vectors, which, through the use of iterators, would have caught the fencepost error in your loop for you. In general, the STL vector is safer and more comfortable than an array while being just as fast and small.
And while were at it, it is best practice to declare your loop variables within the for instead of at the beginning of the function scope (RAII). Unless, of course, you need that value after the loop.
< /niggle >

EDIT 3:
For more on std::string, std::vector and std::otherStuff: the book Accelerated C++ gives a good introduction to C++ and especially the STL. Concepts like char* or arrays only come up quite late, after the STL-way of doing things has been safely covered and practiced. I'd really like to see something like that for the Boost library...

Answer 4

First of all, your for loop is incorrect, it should be like this:

for(i = 0; i < 30; i++)

Second, your array int a[][30] should be like int a[30][] (30 elements of 2, 3 or 4 integers).

Third, you can't pass an array like this, use a pointer.

Well, this code is not clear, and the recursive function could make a stack overflow.

I hope it will help you.

Answer 5

You want to use strcmp instead of == for your string comparison.

However, it's not clear why this would hang. You've removed the code that could have this go into an infinite loop.

Answer 6

No, you are not correctly using C style strings (char *). Give it up and use std::string instead.

For example:

const char *color[30];

for(i=1;i<31;i++)
    color[i]="white";

The above fragment copies pointers and does not make duplicates of the text. This may or may not be what you wanted.

Rewritten more safely:

const char * color[30]; // 30 pointers to char.
const char text_white[] = "white";

for (i = 0; i < 30; ++i)  // Note addition of spaces
{
   color[i] = text_white;
}

The const char text_white[] declares a pointer, text_white, to constant data. This will let the compiler know when you try to assign to the literal.

The C++ code for this:

typedef std::vector<std::string> Text_Container;
Text_Container texts;
const std::string white_text("white");
const unsigned int MAX_TEXTS = 30;
for (i = 0; i < MAX_TEXTS; ++i)
{
    texts[i] = white_text;  // Makes a copy of the string.
}

Give up on char * for text unless your program needs to conserve memory. Prefer std::string and std::vector.