Me myself and I
Me myself and I

Reputation: 57

Better way of doing it? new pointer, delete needed?

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

Answers (2)

alter_igel
alter_igel

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

Deduplicator
Deduplicator

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

Related Questions