-6

I want to dynamically resize the size of an array of chars.

What is the problems with this code ? I got an error on the line "delete[] arr_items[i];"

  char **arr_items; int items=0;
    void AddItem(const char* text) {
    if (items>0) {
      char** tmp = new char*[items+1];
      std::copy(arr_items, arr_items + items, tmp);
      tmp[items] = new char[MAX_LABEL_LEN];
      strncpy(tmp[items], text, MAX_LABEL_LEN);
      for (int i=0; i<items; i++) {
        delete[] arr_items[i];
      }
      delete[] arr_items;
      arr_items = tmp;
    }
    else {
      arr_items = new char*[1];
      arr_items[0] = new char[MAX_LABEL_LEN];
      strncpy(arr_items[items], text, MAX_LABEL_LEN);
    }
    items++;
  }

I don't want to use vector, please refrain, if you know how to do it in the old style, tell me, if not, look elsewhere. The question is clear, if you don't know, keep silence pls!

Caty
  • 63
  • 6
  • 14
    Why not simply use `std::vector` and forget about all of your woes? – πάντα ῥεῖ Jun 10 '23 at 16:53
  • 3
    May I ask where you are learning C++ form? The style of coding you show isn't necessary since 1998 anymore and definitely should not be used since C++11 (12 years ago now). @πάνταῥεῖ Is right, that is the way it should be done. If this is your datastructures teacher then ask him to read the [C++ core guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) and update his material – Pepijn Kramer Jun 10 '23 at 16:57
  • 2
    Good sources to learn cpp from are : A [recent C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) or have a go at https://www.learncpp.com/ (that's pretty decent, and pretty up-to-date). For C++ reference material use : [cppreference](https://en.cppreference.com/w/). And after you learned the C++ basics from those sources, look at the [C++ coreguidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) regularely to keep up-to-date with the latest guidelines. – Pepijn Kramer Jun 10 '23 at 16:58
  • @PepijnKramer I see this popping up a lot, I think most of the professors just don't bother teaching anything else but C++11...I was taught C++ the Java way. And also most of them discourage STL due to learning purposes (2D arrays need to be a pointer to a pointer, arr[] instead of `std::vector`, your own move constructor instead of `std::move()`, etc.) It sucks but the "boss" has the final word right? – underloaded_operator Jun 10 '23 at 17:09
  • 2
    @TheCompile-TimeComedian For me it is like teaching how steam engines work in an age of electric cars... *sighs*. And about datastructures classes : I would first teach how to use datastructes (like std::stack, std::vector, std::list) before letting people have a go at low level implementation themselves. The problem with current teaching is that students will not know the difference between "learning C++" and "using C++ to learn about datastructures". And this does a lot of harm to software industry where we want people who can code in C++ (in a style as described in the C++ core guidelines). – Pepijn Kramer Jun 10 '23 at 17:20
  • 1
    So summarizing, who will teach the teachers. – Pepijn Kramer Jun 10 '23 at 17:21
  • 1
    To answer the question asked, this loop should be removed `for (int i=0; i – john Jun 10 '23 at 17:28
  • But of course all the comments above are correct. This is not the way to write C++ code. But maybe you have no choice in this. – john Jun 10 '23 at 17:29
  • The answer for your question is: [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). I concur with the above sentiments that this is not how modern C++ should be written in the first place. – Eljay Jun 10 '23 at 17:32
  • please show a [mre] – Alan Birtles Jun 10 '23 at 17:35
  • @Caty I apologize if this is more then you asked for. Some of us (including me) are quite passionate about C++ and like to see it used in a way that avoids a lot of bugs. (because that's what code should be all about). – Pepijn Kramer Jun 10 '23 at 17:36
  • There's also a possibility that those calls to `strncpy` are corrupting your data. `strncpy` is not a "safe" replacement for `strcpy`; it has a much lower-level (and much less useful) purpose. If you're worried that somewhere there will be.a string that's too long, think it through: the only way a too-long string could come in would be if it's passed as an argument to `AddItem`; so check it when you add it. Then you **know** that none of the stored strings is too long. – Pete Becker Jun 10 '23 at 17:46
  • john "You are growing the array, there is no reason to delete all the items in the array." very strange what you say, is not possible to "grow", there is a size allocated, only can to delete and allocate the new size. – Caty Jun 10 '23 at 18:23
  • I think here is only novices, no one know how to solve and answer... For novices: in order to consume resources to a minimum and for the speed of execution, the solution given by me is the best, it just needs to be fixed. – Caty Jun 10 '23 at 18:24
  • 2
    *in order to consume resources to a minimum and for the speed of execution* -- Proove that your claim that your code "minimizes resources" and is speedier, please. You are deleting every single item just to add one item. That is neither speedy or minimizes resources. – PaulMcKenzie Jun 10 '23 at 18:27
  • *you don't know, shut up!* -- I suggest you remove this from your post. The comment section is for comments, not for answers. You posted C++ code that is not idiomatic C++, thus comments are expected pointing this out. – PaulMcKenzie Jun 10 '23 at 18:33
  • Pls read the question, I am waiting for an answer only on the subject – Caty Jun 10 '23 at 18:35
  • 1
    @Caty -- bad attitude; most of the folks who have offered comments know far more about C++ than you seem to; and telling people to go away will definitely result in knowledgeable folks ignoring your post. -1. – Pete Becker Jun 10 '23 at 18:36
  • nobody answer how I see – Caty Jun 10 '23 at 18:37
  • 2
    @Caty -- First, is this code part of a C++ class? Why are `arr_items` and `items` global variables? When you say you get an error on the line you stated, please be more specific. A compiler error? A runtime error? If it's a runtime error, post a [mcve] demonstrating how, when, and where you are using this code. Also, you mention you want things done the "old style", but this: `std::copy(arr_items, arr_items + items, tmp);` is an STL algorithm functions, thus is this "old style"? – PaulMcKenzie Jun 10 '23 at 18:43
  • arr_items and items are external to the function and everything in a class. All items must be loaded into array arr_items after successive calls to AddItem() – Caty Jun 11 '23 at 05:57
  • I tried vector, the difference in resources is substantial, vector consumes much more RAM. There is n_items*strings, I don't know how to do it with a char vector, if it is possible. – Caty Jun 11 '23 at 06:00
  • @PepijnKramer Tell me about it, my final exam had 2 pages about STL (initializing vectors, traversing using iterators, what methods are defined for std::map, and so on... The dumbest thing ever if you ask me since all of that is googlable. In a way it makes sense since it is expected to know how the containers are implemented but still – underloaded_operator Jun 11 '23 at 10:45
  • No it is not needed to know how they are implemented, but it is good to know what kind of exists and what you can do with the containers. The rest as you say can be found on google (or better cppreference.com). Anyway don't hesitate to ask more questions if you want to improve your understanding of C++ – Pepijn Kramer Jun 11 '23 at 11:09
  • *vector consumes much more RAM* -- Again, what did you use to make this claim? You mention a "substantial amount", but no proof that this is true. All a `std::vector` is is a class that basically does what your current code attempts to do, but does it without making mistakes. You are calling `new[]`, no different than a `std::vector` would call `new[]`, with maybe the only difference being that vector uses placement-new. – PaulMcKenzie Jun 11 '23 at 14:11
  • Which brings this to the final point -- there is little chance that your code is more efficient than vector, given that you have no "capacity" logic to limit the number of allocations that need to be done when a new string is added. If you add just a single string, you are manipulating the entire set of already-created strings. – PaulMcKenzie Jun 11 '23 at 14:11

1 Answers1

3

What the code does, in essence, is this:

char **items = new char *[1];
items[0] = new char[10];

// resize:
char **new_items = new char *[2];
new_items[0] = items[0];
delete [] items[0];

The problem here is that items[0] is one of the pointers to that character array; it's not the only one. Deleting it destroys the array, and after that, new_items[0] points at nonsense.

In the code in the question, the solution is to remove this loop:

   for (int i=0; i<items; i++) {
    delete[] arr_items[i];
  }

Also, as I mentioned in a comment, strncpy is not appropriate here (or anywhere else).

And, finally, despite your protests, the best solution to this problem is to use std::vector; it manages its own memory and saves you having to do all this manual, error prone stuff. It's extremely unlikely that your home-rolled vector will be smaller and faster than std::vector; that's a C programmer's fantasy.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
  • Thanks but I don't understand what mean items[] and new_items[], I do not use this vars. My delete[] are before new allocation, not after – Caty Jun 11 '23 at 06:02
  • @Caty You do the following steps: 1) allocate new space for all the pointers 2) copy old pointers 3) allocate space for the new string and store the pointer 4) free the space the old pointers point to 5) delete the space for storage where just the old pointers fit. Step 4) is a mistake, because you just copied the pointers, but now you're freeing what they pointed to. You either need to not free the space or you need to copy all strings, not just their pointers if you want to free the old storage (no reason to do this). – PeterT Jun 11 '23 at 06:35
  • Thanks for your explanation! – Caty Jun 16 '23 at 20:14