Thread: String Copy in C++

Why don't you just use the std::string objects and their copy assignment operator=?

Ugh, looking more closely at your code, you are in need of a redesign. Take a look at this line:

Code:

void Phonebook::delFriend(int * index, Phonebook * pb){

Why is index a pointer? You never check for a null pointer, so you might as well use a reference parameter instead. Then, what is the point of pb? This is a member function, but you seem to use pb instead of the *this object. Furthermore, pb appears to be a pointer to the first element of an array. So, you are deleting a friend from an array of phone books, rather than from a phone book?

Next, I see:

Code:

cout<<"Firstname: ";
cin>>fn;
cout<<"Lastname: ";
cin>>ln;

This does not belong here. Your functions should be designed to do one thing and do it well. Hence, separate the concerns: one function deletes the friend; another function requests for and reads the friend name from the user, and then calls the other function to delete the friend (or maybe another function calls both of them).

Now, I observe:

Code:

str.copy(buffer, MAX_NAME_LENGTH);

This is completely unnecessary. You already have str. Why copy to buffer when you don't need such a buffer?

Take a look at this line:

Code:

if((fn.compare(pb[i].get_firstName(i)) ==0) && (ln.compare(pb[i].get_lastName(i)) ==0)){

There is no advantage in using compare here. You can just write:

Code:

if (fn == pb[i].get_firstName(i) && ln == pb[i].get_lastName(i)) {

Then, we come to what you were asking about:

Code:

pb[j+1].get_firstName(j+1).copy(pb[j].get_firstName(j), MAX_NAME_LENGTH);

I am afraid that I cannot redeem this code. What you should do is provide a set_firstName function. Then you can write:

Code:

pb[j].set_firstName(j, pb[j + 1].get_firstName(j + 1));

Actually, this whole business of first names and last names is probably wrong in itself. They should be bunded together in a single phone book entry object.