Getting Garbage Result from dynamically allocated character array in a class

[redfury]

Hey, following is the code of my program.. The code works fine except for the part where it is supposed to print the Name of the customer.
The name of the customer is stored in a dynamically allocated character array in the class...

#include <iostream>
#include <string>
#define _CRT_SECURE_NO_WARNING
using namespace std;

class Cart
{
char* cName;
float total;
static const int cartValue;
public:
Cart();
char* getname();
void setname(string);
void settotal(float);
float gettotal();
Cart(const Cart&);
Cart& additem(float);
void print() const
{
cout << "Customer Name: " << cName << endl;
cout << "Total Purchase Amount: " << total << endl;
}
~Cart();
};
const int Cart::cartValue = 3000;
Cart::Cart()
{
cName = new char[100];
total = 0;
}
float Cart::gettotal()
{
return total;
}
void Cart::setname(string p)
{
int n = p.length();
for (int i = 0; i < n; i++)
cName[i] = p[i];
cName[n] = '\0';
}
void Cart::settotal(float total)
{
this->total = total;
}
char* Cart::getname()
{
return cName;
}
Cart::Cart(const Cart& obj)
{
this->total = obj.total;
this->cName = obj.cName;
}
Cart& Cart::additem(float itemPrice)
{
if ((itemPrice + this->gettotal()) > (this->cartValue))
{
cout << "You don't have enough credits to buy this item.";
return *this;
}
this->settotal(this->gettotal() + itemPrice);
cout << "Item added successfully to the chart";
cout << endl;
return *this;
}
Cart::~Cart()
{
delete[]cName;
}
Cart TakeInput();
bool Check_Input();
int main()
{
Cart obj = TakeInput();
obj.print();
cout << endl << endl;
system("pause");
return 0;
}
Cart TakeInput()
{
Cart obj;
cout << "Enter Customer Name: ";
string name;
getline(cin, name);
obj.setname(name);
cout << endl;
cout << "Your limit of purchase is Rs. 3000/-" << endl << endl;
do
{
cout << "Enter Item Price: ";
float price;
cin >> price;
obj = obj.additem(price);
}while (Check_Input());
return obj;
}
bool Check_Input()
{
cout << "Would you like to add another item (Y/N): ";
char c;
cin >> c;
switch (c)
{
case 'y':
case 'Y':
return true;
default:
return false;
}
}

On running the program, this is the output I get.

Enter Customer Name: John Smith

Your limit of purchase is Rs. 3000/-

Enter Item Price: 55
Item added successfully to the chart
Would you like to add another item (Y/N): Y
Enter Item Price: 555
Item added successfully to the chart
Would you like to add another item (Y/N): N
Customer Name: ▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌
W²ε
Total Purchase Amount: 610


Press any key to continue . . .

As you can see, the customer name is giving garbage value...

I tried different things to solve the issue. For example, I tried using strcpy function, like this

strcpy(this->cName, p); 

However, it gave me the error of CRT SECURE NO WARNINGS... For that, I defined
#define _CRT_SECURE_NO_WARNINGS with the library files, but the error remained intact..

Following the compiler's instructions, then I used strcpy_s to perform the same task.
strcpy_s[this->cName, p)
This didn't repeat the same error but gave a totally new error that strcpy_s doesn't take two arguments, I didn't understand it and I don't know why it gave this error, because I have used it before and it has always worked for me...

Then I opted for the method I have used in my current code (using string), however, this also didn't give me the required output, as described above...

Just to note, I am using Visual Studio 2019... Can anyone help me in this regard, please?

[Ganado]

If you're using C++... just, use, strings. Not char arrays. No strcpy.

char* cName; cName = new char[100];
Don't do this.

Do this:

string name;

void Cart::setname(string p)
{
    name = p;
}

Also,

Cart::Cart(const Cart& obj)
{
this->total = obj.total;
this->cName = obj.cName;
}

This is only copying the pointer of name, not the name itself. Again, if you just use strings, it becomes

Cart::Cart(const Cart& obj)
{
this->total = obj.total;
this->name = obj.name;
}

this correctly copies the string.

If you use strings, you can also get rid of your delete[] in your destructor, because strings are cleaned up automatically.

And in getname, instead of returning a char*, just return a string.

Manual memory management of char arrays is not what you need to worry about right now.

[redfury]

This is the problem.
I am restricted to using char arrays. Otherwise, with the help of strings, this could have been done very easily

[redfury]

This is the problem. I am restricted to using char arrays.

Otherwise, it could have been done using strings very easily

[Ganado]

But you are using strings...?

Well, okay then

First, just like how in your normal constructor, you allocate a char array of size 100, you still need to do in same in your copy constructor, because it's another type of constructor.
Then, also in your copy constructor, you can do something very similar to what you do in your setname function.

this->cName = new char[100];
for (int i = 0; i < 100; i++)
    this->cName[i] = obj.cName[i];

[redfury]

Thanks for the help! ^_^ This fixed the issue... Thank you

[jonnin]

I am not getting this result.
It works perfectly for me. The biggest concern I found is takeitem. Is it destroying that local cart, which would delete the c-string's memory, which is the same pointer that is copied out to its returned value? This is not happening on mine, and I must be not seeing something there, but everything else looks good (though it should use strcpy, and string.c_str(), eg strcpy(cName, name.c_str()); and just lose the whole hands-on manual copy. the manual copy is correct).

any chance you are running a stale executable, and fixed it already?

there may still be a problem, but I haven't been able to spot it and again, it works perfectly for me (not proof that its clean, pointer goofs can do this all too often).

recompile, test, and if it is still broken, replace your dtor with this:
sprintf(cName, "deleted this"); (remove the delete line for now, comment out, it goes back later) and run it like this.

[jonnin]

a little late to the party, I see... nevermind then.