2

I am trying to compile the below code using "g++ main.cpp -c" but it gives me this strange error .. any ideas?

main.cpp: In function ‘int main()’:
main.cpp:9:17: error: invalid conversion from ‘Graph*’ to ‘int’
main.cpp:9:17: error:   initializing argument 1 of ‘Graph::Graph(int)’
main.cpp:10:16: warning: deprecated conversion from string constant to ‘char*’

This is my main module which i am trying to compile and below that is the graph class i have in graph.hpp

#include <iostream>
#include "graph.hpp"

using namespace std;

int main()
{
  Graph g;
  g = new Graph();
  char* path = "graph.csv";
  g.createGraph(path);
  return 0;
}

AND this is my Graph class

    /*
 * graph.hpp
 *
 *  Created on: Jan 28, 2012
 *      Author: ajinkya
 */

#ifndef _GRAPH_HPP_
#define _GRAPH_HPP_

#include "street.hpp"
#include "isection.hpp"
#include <vector>

class Graph
{
 public:
  Graph(const int vertexCount = 0);
  //void addIsection(Isection is);
  //void removeIsection(int iSectionId);
  Isection* findIsection(int);
  void addStreet(int iSection1, int iSection2, int weight);
  void createGraph(const char *path); //uses adj matrix stored in a flat file
  //void removeStreet(int streetID);
  void printGraph();
  ~Graph();
 private:
  //Isection *parkingLot;
  //Isection *freeWay;
  int** adjMatrix;
  std::vector <Street*> edgeList;
  std::vector <Isection*> nodeList;
  int vertexCount;
};

    #endif
ajkl
  • 1,016
  • 1
  • 7
  • 27
  • Consider using `std::string`s for strings, `boost::ptr_vector`s for vectors of pointers, a matrix class for matrices, and not using `using namespace std;` (especially when it serves absolutely no purpose). – Cactus Golov Feb 02 '12 at 01:49

4 Answers4

5

This is C++, not Java or C#. new doesn't work the same way here.

new expressions return pointers. You cannot assign a pointer to a Graph (i.e. a Graph*) to a variable of type Graph:

Graph g;
g = new Graph(); // Graph = Graph* ? nope

Seems like the compiler is trying to be "helpful" and trying to use your constructor take takes an int argument to make a value of type Graph, but it can't convert a Graph* to an int.

When you write Graph g; you already have a Graph object. You don't need to create one with new. In fact, you probably don't even want to do that, as it will lead to memory leaks.

Then there's this line:

char* path = "graph.csv";

"graph.csv" has type char const[10] so you should not assign it to a char*. In the past you could, but that turned out to be a bad idea. That feature was marked as deprecated, and now it was completely removed in C++. Instead of doing that you can:

  • Make an array out of it: char path[] = "graph.csv";;
  • Make a pointer to it with the proper type: char const* path = "graph.csv"; (this works because array types decay to pointers);
Community
  • 1
  • 1
R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
  • Except that, unlike Java, you don't write `Graph g;`, you write `Graph* g;` which is causing his **error**. – Tim Feb 02 '12 at 00:24
  • "*`"graph.csv"` has type `char const[10]` so you cannot assign it to a `char*`*" Morally you shouldn't, but technically you certainly can. @Tim : In idiomatic C++ you certainly _do/should_ write `Graph g;`. – ildjarn Feb 02 '12 at 00:25
  • @Tim - there's no need to make it a pointer and use `new` here at all - this isn't Java after all. – Flexo Feb 02 '12 at 00:26
  • I don't understand. "The new keyword allocates memory for an object or array of objects of type-name from the free store and returns a suitably typed, nonzero pointer to the object." I thought typical usage was `MyClass* instance = new MyClass;` – Tim Feb 02 '12 at 00:29
  • Also, from OP's error: `main.cpp:9:17: error: invalid conversion from ‘Graph*’ to ‘int’` It is clearly showing that his constructor is returning a `Graph*` and he's trying to assign it to a `Graph`. This is throwing the error, correct? – Tim Feb 02 '12 at 00:30
  • @Tim that's typical if you want to do all the memory management yourself, but an `auto` storage object will do just nicely here and not have any worry about leaks/exceptions. You can just write `Graph g;` and be done with it, which avoids a whole world of pain for free. – Flexo Feb 02 '12 at 00:30
  • I did not know that was a thing. Do you have a link to further reading for how it works for me? – Tim Feb 02 '12 at 00:33
  • 3
    I guess working on Objective-C all day is destroying my brain. – Tim Feb 02 '12 at 00:35
  • @Tim - I have exactly the opposite problem in Ojective-C: http://stackoverflow.com/questions/7753680/idiomatic-short-lifespan-local-objects-akin-to-raii – Flexo Feb 02 '12 at 00:39
  • I tried this ` Graph g; char const* path = "graph.csv"; g.createGraph(path);' I am getting these errors now : /tmp/ccmSJigi.o: In function `main': main.cpp:(.text+0x1a): undefined reference to `Graph::Graph(int)' main.cpp:(.text+0x36): undefined reference to `Graph::createGraph(char const*)' main.cpp:(.text+0x42): undefined reference to `Graph::~Graph()' main.cpp:(.text+0x5d): undefined reference to `Graph::~Graph()' collect2: ld returned 1 exit status – ajkl Feb 02 '12 at 01:11
  • @Ajinkya : That means you're not linking graph.o and main.o. How are you building your code? – ildjarn Feb 02 '12 at 01:17
  • yeah my bad .. I did a g++ graph.cpp main.cpp now and even though graph.cpp compiled earlier without errors i get an error on g++ graph.cpp main.cpp saying - In function `Graph::addStreet(int, int, int)': graph.cpp:(.text+0x408): undefined reference to `Isection::pushOutIsectionList(Isection*)' graph.cpp:(.text+0x41a): undefined reference to `Isection::pushInIsectionList(Isection*)' graph.cpp:(.text+0x473): undefined reference to `Isection::pushOutStreetList(Street*)' graph.cpp:(.text+0x485): undefined reference to `Isection::pushInStreetList(Street*)' Can i share the files here somehow ? – ajkl Feb 02 '12 at 01:21
  • @Ajinkya Are you forgetting the `-c` flag? – R. Martinho Fernandes Feb 02 '12 at 01:28
  • i am doing a g++ graph.cpp -c and then a g++ graph.cpp main.cpp – ajkl Feb 02 '12 at 01:32
  • @Ajinkya: ah, I see. After you compiled all the parts (including main.cpp) with `-c` you should see a bunch of files ending in `.o` in your directory. The final step should be with *all* of those `.o` files. Like `g++ graph.o isection.o main.o etc.o`. – R. Martinho Fernandes Feb 02 '12 at 01:38
  • got you! thanks for the help!! i am new to c++ and wanted to use g++ for compiling rather than any gui based compiler. This makes the learning more fun and clearer. – ajkl Feb 02 '12 at 01:50
2

That should probably be g = Graph(); and forget the g = new Graph;. The reason is that new returns a pointer to the created object (e.g. a Graph*), not an object value.

Better still, just do Graph g; and forget about assigning anything to g. This will automatically create a Graph for you by calling Graph's no-arg constructor.

Mac
  • 14,615
  • 9
  • 62
  • 80
0

Graph* g;

It has to be a pointer.

Tim
  • 14,447
  • 6
  • 40
  • 63
  • 1
    For a bit more explanation: the only viable constructor for `Graph` is the one taking an `int`. This could be a conversion constructor as it isn't `explicit` but this conversion can't be used as the argument type mismatch. You might want to make your constructor `explicit` to improve error messages for situations like the one quite and to avoid accidentally creating a `Graph`. – Dietmar Kühl Feb 02 '12 at 00:21
  • His constructor has a default value of 0: `Graph(const int vertexCount = 0);` therefore `Graph()` = `Graph(0)` – Tim Feb 02 '12 at 00:22
  • It can still be be `explicit`, however. My point was just to explain where the odd error message about an `int` to `Graph` conversion came from. – Dietmar Kühl Feb 02 '12 at 00:31
  • 1
    Insisting on making everything a pointer and using `new` to initalise (or assign values to) them is a very Java-esque way of thinking, which doesn't map directly onto C++ as well. – Flexo Feb 02 '12 at 00:32
  • If you advocate for it be a pointer, please at least add something about it being a smart pointer (or that it must be explicitly deleted and that throwing exceptions will probably cause memory leaks/drive the author insane). – Cactus Golov Feb 02 '12 at 01:52
0

First of all new Graph() returns a Graph*. In c++, a constructor with one argument can be implicitly called, so currently your code is really meaning:

g = Graph(new Graph());

What you want is

g = Graph();
JKor
  • 3,822
  • 3
  • 28
  • 36
  • This will needlessly assign to `g` the exact same value it already has. How is that an answer? – ildjarn Feb 02 '12 at 01:18
  • When I learned C++ I was taught to always initialize my variables myself and never leave it to the system or the compiler. I also was fixing his problem as it was and explaining what is current code was actually doing. – JKor Feb 03 '12 at 03:32
  • All I can say is that if that's true, you were taught very poorly. This defines [cargo cult programming](http://en.wikipedia.org/wiki/Cargo_cult_programming). – ildjarn Feb 03 '12 at 03:52