0

I tried to run the bwlow code, but I am getting an error

'this' cannot be used in a constant expression

I need to implement a function that received intervals graph with coloring and also calculate the edges and max\minimum degree, but when I try to run the program I have a problem with this->V. I need help, how can I fix this? I tried with vector but was unsuccessful. the error appear here int result[V];

i tried to change to vector but i getting this error enter image description here

#include <iostream>
#include <list>
using namespace std;

// A class that represents an undirected graph
class Graph {
    int V; // No. of vertices
    int** adj; // 2D adjacency matrix
public:
    // Constructor and destructor
    Graph(int V);
    // ~Graph()  { delete [] adj; }

    void addEdge(int v, int w); // function to add an edge to graph
    void greedyColoring(); // Prints greedy coloring of the vertices
};

Graph::Graph(int V)
{
    this->V = V;
    adj = new int*[V];
    for (int i = 0; i < V; i++) {
        adj[i] = new int[V];
    }

    for (int i = 0; i < V; i++) {
        for (int j = 0; j < V; j++) {
            adj[i][j] = 0;
        }
    }
}

void Graph::addEdge(int v, int w)
{
    adj[v][w] = 1;
    adj[w][v] = 1; //undirected graph
    |
}

// Assigns colors (starting from 0) to all vertices and prints
// the assignment of colors
void Graph::greedyColoring()
{
    int result[V];

    // Assign the first color to first vertex
    result[0] = 0;

    // Initialize remaining V-1 vertices as unassigned
    for (int u = 1; u < V; u++)
        result[u] = -1; // no color is assigned to u

    // A temporary array to store the available colors. True
    // value of available[cr] would mean that the color cr is
    // available to assign
    bool available[V];
    for (int cr = 0; cr < V; cr++)
        available[cr] = true;

    // Assign colors to remaining V-1 vertices
    for (int u = 1; u < V; u++) {
        // Process all adjacent vertices and flag their colors
        // as unavailable
        for (int i = 0; i < V; i++) {
            if (adj[u][i]) {
                if (result[i] != -1) {
                    available[result[i]] = false;
                }
            }
        }

        // Find the first available color
        int cr;
        for (cr = 0; cr < V; cr++)
            if (available[cr] == true)
                break;

        result[u] = cr; // Assign the found color

        // Reset the values back to true for the next iteration
        for (int cr = 0; cr < V; cr++)
            available[cr] = true;
    }

    // print the result
    for (int u = 0; u < V; u++)
        cout << "Vertex " << u << " ---> Color "
             << result[u] << endl;
}

// Driver program to test above function
int main()
{
    Graph g1(5);
    g1.addEdge(0, 1);
    g1.addEdge(0, 2);
    g1.addEdge(1, 2);
    g1.addEdge(1, 3);
    g1.addEdge(2, 3);
    g1.addEdge(3, 4);
    cout << "Coloring of graph 1 \n";
    g1.greedyColoring();
}
danielshmu shmayovich
  • 65
  • 1
  • 6
  • 3
    [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) / Use `std::vector`. – Evg Jan 08 '20 at 18:02
  • 8
    It's probably easier if you tell us **where** the error occurs, instead of us reading 100 lines of code and trying to guess. – super Jan 08 '20 at 18:02
  • 1
    To be fair, he only uses `this` on a single line in the first block. – alteredinstance Jan 08 '20 at 18:04
  • Why even use `this`? Just use a different name for your argument instead of the exact same name as your member variable. `Graph::Graph(int V)` -> `Graph::Graph(int newV)`. The compiler has no idea which `V` is the function argument and which is the member variable in that function. – alteredinstance Jan 08 '20 at 18:06
  • 2
    Please edit your question to contain [mcve] and provide exact error message and which line produced it – Slava Jan 08 '20 at 18:09
  • 2D dynamic allocation – Lightness Races in Orbit Jan 08 '20 at 18:19
  • @alteredinstance And that line isn't a constant expression – Lightness Races in Orbit Jan 08 '20 at 18:20
  • _"I tried with vector but was unsuccessful"_ In what way were you unsuccessful? – Lightness Races in Orbit Jan 08 '20 at 18:21
  • 1
    @alteredinstance I expect it is an implicit usage of `this` because there is nothing wrong with the line `this->V = V;` – user253751 Jan 08 '20 at 18:27
  • i tried this in gcc and clang and it compiles except for the extra pipe `|` you left in `addEdge()`. no compile errors... – Cruz Jean Jan 08 '20 at 18:29
  • 1
    *I tried with vector but was unsuccessful* -- [Maybe you just did not use vector correctly](http://coliru.stacked-crooked.com/a/1541e6a32cf83e0f). Whether or not it is the right answer, that's a different issue. – PaulMcKenzie Jan 08 '20 at 18:32
  • @CruzJean: I believe gcc and clang support VLAs as a language extension. They'll at least warn with a high enough warning level, though. – Fred Larson Jan 08 '20 at 18:35
  • @FredLarson Ah, yes. I didn't notice that was being used. And apparently gcc/clang only warn this with `-Wpedantic` (not even with `-Wall`). – Cruz Jean Jan 08 '20 at 18:38
  • @CruzJean: I have `-Wall -Wextra -pedantic`, which is certainly enough. The warning I get contains `[-Wvla]`, so presumably that flag would do it too. – Fred Larson Jan 08 '20 at 18:39
  • @LightnessRacesBY-SA3.0 The compiler clearly thinks `this->V = V;` is a constant expression with an illegal `this` inside of it, lol. It's probably getting confused as hell because OP has named his member and function variables the exact same `V` – alteredinstance Jan 08 '20 at 18:46
  • [Quick and surprisingly clean matrix class](https://stackoverflow.com/a/2076668/4581301) that makes a good replacement for `int** adj;` – user4581301 Jan 08 '20 at 18:46
  • 1
    @alteredinstance That's not clear at all. There is no reason for it to do that, and there is no problem with the names. See rustyx's answer: the problem has nothing to do with the line you identified, which proves that your notion we only need to look at one line is false. – Lightness Races in Orbit Jan 08 '20 at 18:48
  • @LightnessRacesBY-SA3.0 He said in his answer that the compiler is expecting a constant expression there - which is exactly what I said, even though I didn't diagnose the problem. It was also never my notion that "we only need to look at one line". I pointed out a clearly bad coding practice - I was wrong about the compiler being confused, it must just use the `V` within the scope of the function. – alteredinstance Jan 08 '20 at 18:59
  • @alteredinstance in response to someone asking where the error was, you said _"To be fair, he only uses this on a single line in the first block."_ The error wasn't on that line. No need to go over this any more though – Lightness Races in Orbit Jan 08 '20 at 23:27
  • @alteredinstance _"it must just use the V within the scope of the function"_ The name of the argument is found before looking at the names of members. That's why `this->` is required on that line – Lightness Races in Orbit Jan 08 '20 at 23:27

1 Answers1

3

The error is probably not on the line you mentioned but here

int result[V];

Why? Because standard C++ does not support variable-length arrays, so V, which is actually this->V, is expected to be a constant expression, which it isn't.

Use a vector instead:

std::vector<int> result(V);

Don't forget to #include <vector>.

Why your code compiles with GCC? Because GCC supports variable-length arrays as an extension.

rustyx
  • 80,671
  • 25
  • 200
  • 267