C++ Compiler Error: No matching function for call

Question

Look at the following code. What is wrong with it? The compiler gives the this error:

In copy constructor person::person(person&)': No matching function for call toperson::copy(char*&, char*&)' candidates are: void person::copy(char*&, const char*&) "

Here is the code:

class person
{
  public:
    person();
    person(person &);

  private:
    void copy(char*&,const char*&);
    char* name, *fathername,* address;
};

void person::copy( char*& n, const char*& p)
{
  int result;
  result=strcmp(n,p);
  if(result!=0)
  {
    n=new char[strlen(p)+1];
    strcpy(n,p);
    n[strlen(p)]='\0';
  }
}
person::person(person &object)
{
  copy(name,object.name);
  copy(fathername,object.fathername);
  copy(address, object.address);
}

From the answers to this Question what I understood up until now is given by: the compiler does not allow to convert a reference to a constant reference because references are already constant. They can't point to a different memory location like pointer. Am I right?

Solution 1

Wouldn't this be nicer?

class person
{
private:
  std::string name;
  std::string fathername
  std::string address;
};

// constructor and copy constructor autogenerated!

It's more "C++" this way ;).

Solution 2

Unless you are planning on changing the pointers you should not pass the references to the pointers:

Change:

void person::copy( char*& n, const char*& p) 

to

void person::copy( char* n, const char* p) 

This is because p is a reference to a particular type.
The object you passed is not the exact type and becuase it is a reference their is no way to convert it.

The change I suggested above allows for a "pointer to const char" (p) thus allowing read only access to elements via 'p'. Now a "pointer to char" allows read/write access to the data so converting this to "pointer to const char" is allowed because we are just limiting the allowed behavior.

There are a whole set of other problems with the code you posted.
Do you want us to list them?

I don't do NOW. I do on my schedule.

Problems:

1: You leak on each call to copy:

if(result!=0)
{
    n=new char[strlen(p)+1];   // What happned to the old n?

2: The default assignment operator is used.

person a;
person b;
a = b; // a.name == b.name etc all point at the same memory location.
       // Though because you do not delete anything in the destructor
       // it is technically not an issue yet.

3: You done delete the allocated members in the destructor.

{
     person  a;
} // A destructor called. You leak all the member here.

4: strcpy() already copies the terminating '\0' character.

5: if the call to new throws an exception. You will leak memory.

copy(name,object.name); 
copy(fathername,object.fathername);   // If new throws in here.
                                      // Then the this.name will be leaked.

Doing this correctly using C-String is so hard that even a C++ expert would have problems doing this correctly. Thats why C++ experts would use std::string rather than a C-String. If you must use C-Strings then you should wrap your C-String in another class to protect it from probelms with exceptions.

Solution 3

Change

person::person(person &object)

to

person::person(const person &object)

for starters...

Solution 4

The compiler is telling you the problem - change your signature to accept 2 char* pointers (rather than 1 const char*) and it should compile.

The issue is really due ot the use of the reference - if you had created a copy method that simply took 2 char* pointers (not references) then the compiler will automatically recognise the conversion from char* to const char* and use the method. As you only have a method that accepts a reference to a different type it cannot do that automatically.

Solution 5

I'm feeling generous, so here is a corrected version of your code:

class person
{
public:
    person();
    person(const person &);
    ~person();
private:
    void copy(char*&,   // Do you understand purpose of '&' here?
              const char*);
    char* name;
    char* fathername;
    char* address;
};

person::person()
    : name(NULL),
      fathername(NULL),
      address(NULL)
{
}

person::~person()
{
    delete[] name;
    delete[] fathername;
    delete[] address;
}

void person::copy( char*& n,  // The '&' is required because the contents of `n` are changed.
                   const char* p)
{
    delete[] n;
    n = NULL;     // Here is one place where contents of `n` are changed.
    if (p)
    {
        n = new char [strlen(p) + sizeof('\0')];  // Another content changing location.
        strcpy(n, p);
        n[strlen(p)]='\0';
    }
}

person::person(const person& object)
{
    copy(name,object.name);
    copy(fathername,object.fathername);
    copy(address, object.address);
}

Can you identify the flaws or safety items still lurking?

View more solutions

Comments

[Zia ur Rahman]

please explain in understanable manner , i could not understand it

[Zia ur Rahman]

ya its nice but the nature of the problem is different

[Yogesh Arora]

hey its good to use strings, but from the type of question he has asked , i think he is more of a beginner in c++/oo programming . It better to clear your concepts like this..rather than directly jumping on to string

[Zia ur Rahman]

this is the answer to my question , but i need explanation of this answere

[Kornel Kisielewicz]

@Yogesh - this is actually a matter of principle -- people comming from low-level language backgrounds tend to think that std::string's are bloated and ineffective in usage, that's why despite using C++ they use char*. In each such situation someone should point out that there's std::string...

[Kornel Kisielewicz]

@Yogesh - also, the value of learning to use char* before std::string when learning C++ is at least controversial -- there are two schools of that thought, and you shouldn't assume that one is the only true.

[Zia ur Rahman]

please any body explain the answer of the person gbjbaanb

[Martin York]

Why would you do that. If all the member variables are std::string then compiler generated copy constructor works just fine, you should not be writting your own.

[Thomas Matthews]

See @Martin York's answer below. Remember, only use & with pointers (e.g. char *) when you will modify the pointer.

[Zia ur Rahman]

ya you should list all the problems without asking

[Zia ur Rahman]

your answer is helpful, compiler is not allowing to convert a reference to a constant reference because references are already constant, they can't point to a different memory location like pointer am i right???????

[Zia ur Rahman]

ya i know all these , as i have previously written this is not the actual code which i posted , i posted only that code which has problem, i removed the code of destructor and constructors because of simplicity, because you know people hate to read lengthy code.

[Bill]

Ooh! Ooh! It's not self-assignment safe. :)

[gbjbaanb]

+1 for "I don't do NOW. I do on my schedule.", cheers for explaining my answer in such a complete and nice way too.