-2

I'm learning C++ by making a Graph class (with Vertex and Edge class as well). I'm having A LOT of errors:

Error   72  error C2039: 'other' : is not a member of '`global namespace''  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 167
Error   92  error C2039: 'pointer' : is not a member of 'std::_Wrap_alloc<_Alloc>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\vector   450
Error   98  error C2039: 'pointer' : is not a member of 'std::_Wrap_alloc<_Alloc>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 104
Error   102 error C2039: 'pointer' : is not a member of 'std::_Wrap_alloc<_Alloc>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\vector   458
Error   70  error C2039: 'rebind' : is not a member of '`global namespace'' c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 167
Error   15  error C2039: 'type' : is not a member of 'std::_Get_pointer_type<_Ty>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 421
Error   18  error C2039: 'type' : is not a member of 'std::_Get_pointer_type<_Ty>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 248
Error   27  error C2039: 'type' : is not a member of 'std::_Get_pointer_type<_Ty>'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 255
Error   59  error C2146: syntax error : missing ';' before identifier 'allocate'    c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 874
Error   84  error C2770: invalid explicit template argument(s) for '_Uty::void_pointer std::_Get_void_pointer_type<_Ty>::_Fn(int)'  c:\program files (x86)\microsoft visual studio 11.0\vc\include\xmemory0 255

Those are only some of them, there are a lot more (these are in Visual Studio 2012). I have some in Code Blocks as well (I'm trying to debug with both of them)). Here are some of Code Blocks:

error: no matching function for call to 'make_heap(std::vector<std::pair<Vertex<int>*, double>, std::allocator<std::pair<Vertex<int>*, double> > >::iterator, std::vector<std::pair<Vertex<int>*, double>, std::allocator<std::pair<Vertex<int>*, double> > >::iterator, <unresolved overloaded function type>)'
note: candidates are:
note: template<class _RAIter> void std::make_heap(_RAIter, _RAIter)
note:   template argument deduction/substitution failed:
note:   candidate expects 2 arguments, 3 provided
note: void std::make_heap(_RAIter, _RAIter, _Compare) [with _RAIter = __gnu_cxx::__normal_iterator<std::pair<Vertex<int>*, double>*, std::vector<std::pair<Vertex<int>*, double>, std::allocator<std::pair<Vertex<int>*, double> > > >; _Compare = bool (Graph<int>::*)(std::pair<Vertex<int>*, double>, std::pair<Vertex<int>*, double>)]

Here are some snippets of my code that could possibly point to the error's source (I'm not putting all my code here because I saw a topic in Help section saying I shouldn't put too much code, only what is needed):

template <typename T>
class Graph
{
private:
    std::vector< Vertex<T>* > m_vVertexes;
    int m_nIndexGenerator;

    Graph(const Graph& g);
    Graph& operator=(const Graph& g);
    void Init();
    bool Compare(std::pair< Vertex<T>*, double > pFrom, std::pair< Vertex<T>*, double > pTo);

template <typename T>
inline bool Graph<T>::Compare(std::pair< Vertex<T>*, double > pFrom, std::pair< Vertex<T>*, double > pTo)
{
    return pFrom.second >= pTo.second;
}

template <typename T>
bool Graph<T>::A_Star(int source, int destiny, double (* Heuristic)(const T& tFrom, const T& tTo), const bool testIfItsOnOpenSet)
{
    int vSize = m_vVertexes.size();
    bool found = false;

    // Checking if it is not out of the bounds
    if(source < 0 || destiny < 0 || source >= vSize || destiny >= vSize)
        return false;

    Vertex<T>* vDestiny = m_vVertexes[destiny];
    T& tDestinyInfo = vDestiny->GetInfo();

    // Initializing the vectors to proper values
    Init();
    m_vVertexes[source]->SetCost(0.0);
    std::pair< Vertex<T>*, double > pTempFrom;

    pTempFrom.second = Heuristic(m_vVertexes[source]->GetInfo(), tDestinyInfo);
    pTempFrom.first = m_vVertexes[source];

    // Initializing the minHeap
    std::vector< std::pair< Vertex<T>*, double > > minHeap;
    minHeap.reserve(vSize);

    std::make_heap(minHeap.begin(), minHeap.end(), Compare);
    minHeap.push_back(pTempFrom);
    std::push_heap(minHeap.begin(), minHeap.end(), Compare);

    while(!minHeap.empty())
    {
        std::pop_heap(minHeap.begin(), minHeap.end(), Compare);
        pTempFrom = minHeap.back();
        minHeap.pop_back();

        Vertex<T>* vTempFrom = pTempFrom.first;
        vTempFrom->SetColor(Black);

        double dCostFrom = vTempFrom->GetCost();

        if(vTempFrom == vDestiny)
        {
            found = true;
            break;
        }

        int fromIndex = vTempFrom->GetIndex();

        for(int i = vTempFrom->GetNextAdjacentVertex(); i > -1; i = vTempFrom->GetNextAdjacentVertex())
        {
            Vertex<T>* vTempTo = m_vVertexes[i];

            double dCost = dCostFrom + vTempFrom->GetEdgeCost(vTempTo->GetIndex);

            if(testIfItsOnOpenSet && vTempTo->GetColor() == Gray && dCost < vTempTo->GetCost())
            {
                vTempTo->SetColor(White);
                typename std::vector< Vertex<T>*, double >::iterator it = std::find(minHeap.begin(), minHeap.end(), vTempTo); // I only put this typename in there because CodeBlocks was complaining about it, I didn't get it though
                minHeap.erase(it);
                std::make_heap(minHeap.begin(), minHeap.end(), Compare);
            }

I dind't post everything and I didn't even post the entire A_Star algorithm, because I think the mistake is somewhere in what I posted.

Note: all that code is inside a .h file, because if I had the definition in a .cpp file I would have to say which types I wanted for template typename (at least that's what I understood).

@edit

Here is the link to see the entire code if needed: https://github.com/Jordan2R/GraphDebug

YardenJ2R
  • 9
  • 1
  • 4
  • compile with `-Wfatal-errors` (or equivalent of your compiler) to only get the first error. Or alternatively, just ignore any error, but the first one. All other errors may not be actual errors. – stefan Jan 11 '15 at 14:50
  • It seems like you haven't closed your Graph class with a '};' - either that or it's further down and not in what you posted. Also, if you're defining those member functions in the class body they shouldn't be getting an additional template specification since they already have the class template specification. – mbgda Jan 11 '15 at 14:58
  • Your `Compare` function is a regular member function, so it cannot be directly used as the comparator. Make it static member function and try again. – Wojtek Surowka Jan 11 '15 at 14:59
  • Drop the whole `template ...` stuff, it only complicates things. Once you have that running, you can add the template stuff back in. – Ulrich Eckhardt Jan 11 '15 at 18:42
  • I do have the Graph class closed, I only posted a snippet because I read on Help section that it was not good to post all code (too much code it seems). I'm only declaring the member functions on the class body, they are defined later. I tried with static already (tried again only because you asked). Ulrich, sorry man I want to make it work with template. Actually, A lot of stuff is working (there a lot to be done yet). In my main.cpp, if I don't call the A_Star algorithm, everything works. So the class (until now) is kind of OK. All code: https://github.com/Jordan2R/GraphDebug – YardenJ2R Jan 11 '15 at 21:16
  • 3
    What you should try is to make a short complete example that demonstrates the issue. Basically, take what you have and start removing code that isn't causing the problem until you have a small example that does. – Retired Ninja Jan 11 '15 at 21:33
  • 2
    I agree with @RetiredNinja. In particular, it troubles me that you can post a question with the code in a stage with so many _independent_ issues. Really, you need to let your compiler help out much more often, much earlier, and fix the problems as soon as they pop up. See also **[Nobody Writes Testcases Anymore](http://kera.name/articles/2013/10/nobody-writes-testcases-any-more/)** and **[Solve your problem by _almost_ asking a question on Stackoverflow](http://blog.jerryorr.com/2014/04/solve-your-problem-by-almost-asking.html)** – sehe Jan 11 '15 at 22:02
  • Man, whenever I create a small functional part of the program I test it. Everything was working until I finished A_Star and called it on main.cpp, which has few lines. There isn't an example to show. If I didn't call it, everything worked (i.e., creating Vertexes and Edges). If I called A_Star, bang! And that's why I just showed the snippet (as said in http://stackoverflow.com/help/how-to-ask, "don't just copy in your entire program (...), it likely includes a lot of irrelevant details that readers will need to ignore when trying to reproduce the problem"). – YardenJ2R Jan 12 '15 at 00:04
  • Is the `templace` stuff necessary to demonstrate the issue? If yes, say so, if no, remove it before posting. *THAT* is the "irrelevant details" that the howto talks about. However, I guess that you are not aware that lots of template code can be written that doesn't make any sense at all. It may be syntactically valid, but unless the template is instantiated (which is what A* probably does) it isn't checked whether it makes sense. Anyway, if you don't want help and rather learn it the hard way, ignore all good suggestions made here. – Ulrich Eckhardt Jan 12 '15 at 19:30
  • Of course I want help, I came here for that reason. I don't want to learn the hard way, if there an easy way, please show me one. I'm reading everything and accepting the good suggestions, I didn't get why you are saying those things. And the bugs have already been fixed with the help – YardenJ2R Jan 14 '15 at 11:36

1 Answers1

4
  1. You missed parens here:

    double dCost = dCostFrom + vTempFrom->GetEdgeCost(vTempTo->GetIndex());
                                                                       ^^^
    
  2. You mistyped the type here (note also I added typename which MSVC erronously doesn't require):

    typename std::vector< Vertex<T>*, double >::iterator it = std::find(minHeap.begin(), minHeap.end(), vTempTo);
    

    That double is specified where the allocator is expected leading to the host of errors you see. Fix it to be the required type and it's gone:

    typename std::vector< std::pair<Vertex<T>*, double > >::iterator it = std::find(minHeap.begin(), minHeap.end(), vTempTo);
    
  3. Sadly that exposes a new problem: you're searching in a vector of those pairs by "just a Vertex<T>* const". That won't work, because you can't compare them. If this is really what you wanted, add the operator== overload, but in reality it looks like you wanted

    std::map<Vertex<T>*, double> 
    

    instead of that vector. I just added this to progress compilation:

    // FIXING ERRORS IN "GUESS" MODE
    template <typename T>
    bool operator==(std::pair<Vertex<T>*, double> const& p, Vertex<T> const* v) {
        return p.first == v;
    }
    
  4. Finally, GetCost doesn't have external linkage, so you get:

    || /tmp/cc36WPls.o: In function `Vertex<int>::GetEdgeCost(int)':
    Vertex.h|127| undefined reference to `Edge::GetCost()'
    

    Remove inline from the implementation file

     /*not inline*/ double  Edge::GetCost() { return m_dCost; }
    

    Minor things:

  5. Chaining with -> cannot be done on Vertex& objects. Use . instead of ->

  6. Specifying static on static class member functions is illegal when defining the body out-of-class

It now compiles.

I've made a pull request so it's easier to review the changes: https://github.com/Jordan2R/GraphDebug/pull/1

  • 1b23cd3 Fix initialization/declaration order (may I suggest -Wall -pedantic)
  • 6b29461 Making the find in A_Star compile
  • ca09211 Missing parens
  • ce7c6c1 Minor issues
  • ae349c4 Donot abuse inline (it messes up external linkage)
sehe
  • 328,274
  • 43
  • 416
  • 565
  • Oh, I noticed that GetCost was implemented, but not with correct linkage. Updated under 4. – sehe Jan 11 '15 at 22:27
  • You went above and beyond the call of duty on this one. :) – Retired Ninja Jan 11 '15 at 23:35
  • Wow sehe, it worked! Thanks man! Thanks to all that answered! But I didn't get some stuff. About the mistake number 1, that was silly of me, forgetting the parentheses. Sorry about that! About number 2, why typename is needed? I didn't get it. About number 3, I didn't want to use std::map actually. And once again, I didn't understand what you meant. About number 4, I didn't understand why there was a linkage problem. Weren't the includes correctly made? About number 5, silly me again. Sorry about that. About number 6, so if I want static methods in C++ I write it on the declaration only? – YardenJ2R Jan 11 '15 at 23:56
  • What you meant by GUESS MODE? That you were guessing the answer to the error? – YardenJ2R Jan 12 '15 at 00:19
  • Ok! **1.** no need to apologize **2.** [dependent names](http://en.cppreference.com/w/cpp/language/dependent_name) require it: [where and why do I have to put the template and typename keyword](http://stackoverflow.com/questions/610245/where-and-why-do-i-have-to-put-the-template-and-typename-keywords) **3.** I was guessing this because the `find` suggests you do a lookup by key; (multi)maps have this feature **4.** [the FAQ has an intro](http://www.parashift.com/c++-faq-lite/inline-nonmember-fns.html), search SO for more detailed background 6. indeed, for static class members – sehe Jan 12 '15 at 07:22
  • "GUESS MODE" means that I don't claim the /behaviour/ of the code is what you need. You were "comparing apples to pears": it looked (to me) that you might have wanted to look up the double "by" the vertex as key. Since I didn't _read_ what your code was trying to achieve, this is _an assumption_ and you should _check_ that it actually does what you need. Obviously, the compiler is "content" now, but I could have made the compiler content in other ways (deleting the code, e.g.). I don't claim anything about **semantic** correctness. – sehe Jan 12 '15 at 07:28
  • Oh now I get it...find is calling that operator. I was silly again, sorry. What I actually wanted was something like: typename std::vector< std::pair*, double > >::iterator it = std::find(minHeap.begin(), minHeap.end(), std::pair*, double >(vTempTo, dCost)); Well, thanks again! There's a lot for me to learn yet @edit I don't know why, I'm not being able to transform that sentence into code – YardenJ2R Jan 12 '15 at 12:06
  • Wokay. In that case, one more link for you to read: ["What every computer scientist should know about floating-point arithmetic"](http://download.oracle.com/docs/cd/E19957-01/806-3568/ncg_goldberg.html). Specifically [Most effective way for float and double comparison](http://stackoverflow.com/questions/17333/most-effective-way-for-float-and-double-comparison). You might consider fixed point or decimal representations – sehe Jan 12 '15 at 13:42
  • Yeah man, I know some floating-point airthmetic and comparison, but I will check your links. Usually I use integers, but for this Graph I decided using floating-point. Thanks! – YardenJ2R Jan 14 '15 at 11:34