1

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));
}
  • 4
    *"I'm wondering if this is a good or bad way of using pointers and new."* There is no good way of using raw pointers and new. And smart pointers and new are rarely necessary, too. In this case, you'd want `std::string` instead of pointers. – Baum mit Augen Mar 17 '18 at 20:09
  • 1
    Why not `std::string`? – vishal-wadhwa Mar 17 '18 at 20:10
  • Without using string, forgot to mention (because of my teacher). – Me myself and I Mar 17 '18 at 20:10
  • 3
    Then your teacher is not teaching you C++. The mighty Spaghetti Monster knows what he/she is teaching you. – Ron Mar 17 '18 at 20:11
  • He is, but we are not going to learn string as of yet. – Me myself and I Mar 17 '18 at 20:12
  • 1
    Supplement your formal education with these [C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – Ron Mar 17 '18 at 20:13
  • @BaummitAugen: There is a good way to use raw memory-management: In the implementation of `std::unique_ptr` and support, maybe even other basic abstractions. Granted, that is rare. – Deduplicator Mar 17 '18 at 20:17
  • 2
    @Deduplicator Ok, granted, slight overgeneralization there. I'd be willing to throw in a "for people that are not implementing resource handlers". – Baum mit Augen Mar 17 '18 at 20:24
  • No teacher can prevent you from learning std::string. – Jive Dadson Mar 17 '18 at 21:07
  • Not prevent me from learning, but prevent me from using it when I only can use stuff that he has taught. – Me myself and I Mar 17 '18 at 21:16
  • If your teacher has started out teaching C style arrays, pointers, and `new`, and particularly if he will not allow you to discover and use features he has not taught, you are going to need to practice defensive learning. Know that the code that will get you A's is going to be lousy code. – Jive Dadson Mar 17 '18 at 21:17
  • 1
    Also, it is not going to be fun. It should be fun. – Jive Dadson Mar 17 '18 at 21:20
  • Seems like the teacher believes you should learn to run a simple [unicycle](https://www.unicycle.uk.com/20-hoppley-beginner-unicycle.html) first, and then move on to the "advanced" two- and [three-wheelers](https://www.amazon.com/Radio-Flyer-Red-Classic-Tricycle/dp/B00BPKPCGG) later. That's not really the right order. – Bo Persson Mar 17 '18 at 23:11

2 Answers2

3

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.

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
3

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++.

alter_igel
  • 6,899
  • 3
  • 21
  • 40