Reputation: 57
I'm wondering if this is a good or bad way of using pointers and new. I don't want to store more than I need, so therefore I make a buffer of 50 and the user write their username, and I use strlen to count it and store the length + 1. Do I have to delete username afterwards? Should I do this different?
Without using string.
void Accounts::newUser()
{
char buffer[50]; char *username;
cout << "username: "; cin.getline(buffer, 50);
username = new char[strlen(buffer) + 1]; strcpy(username, buffer);
// User(char *username) <- parameter
accountList->add(new User(username));
}
Upvotes: 1
Views: 63
Reputation: 7212
Firstly, this code reads more like C than C++. And in good, modern C++, you shouldn't really need to use new
and delete
, since it is very error prone and the standard library offers all the same behaviour in a much more maintainable manner, for example using std::string
and std::shared_ptr
and value semantics.
In this example, speaking in terms of C-style code, the correct usage of pointers is unclear from what is given. It depends on whether the User
constructor takes ownership of the pointer it is passed, or if it copies the string to an internal member.
If User
takes ownership, then you should not delete username
in this function, but you should also ensure that User
correctly deallocates its memory properly in its destructor. Otherwise, if User
does not take ownership of the pointer, username
is not necessary, and passing in buffer
to the constructor of User
should suffice.
That said, your code mixes C-style and C++, which is bad practice. I recommend choosing between learning simply C and learning modern C++.
Upvotes: 3
Reputation: 45674
Well, your use of new
is abysmal because it ensures leaks if the second raw new
throws.
Use a std::string
for your name-variable (or at least some way to safely juggle strings), and at least use std::make_unique<User>(username)
for the second one. You have to fix User
and Accounts
too.
Thus, there is no risk of leaks anymore.
Upvotes: 3