C++ dynamic allocated array
I'm doing some assignment and got stuck at one point here. I am trying to write an list_add() function. The first functionality of it is to add values to the array. The second functionality for it is to increase the size of the array. So it works much like a vector. I dunno if I get it right though. What I tried is to create a new dynamic allocated array which is larger than the old one, and then copy over all the values to the new array.
Is it the right approach?
Here's the main body
int main()
{
const int N = 7;
//declaring dynamic array allocation
int* list = new int[开发者_开发百科N];
int used = 0, a_val;
for(int i=0;i<11;i++)
{
list_add(list, used, N, i);
}
cout << endl << "Storlek: " << N << endl << endl;
cout << "Printar listan " << endl;
for(int i=0;i<used;i++)
{
cout << list[i] << ". ";
}
}
Here's the function
bool list_add(int *list, int& space_used, int max_size, int value)
{
if(max_size-space_used > 0)
{
*(list+(max_size-space_used-1)) = value;
space_used++;
return true;
}
else
{
cout << "Increasing size of array!" << endl;
int new_max_size = space_used+1;
delete [] list;
int *list_new = new int[new_max_size];
for(int i=0; i<new_max_size; i++)
{
list_new[i] = i;
cout << list_new[i] << ". ";
}
cout << endl;
space_used++;
list = list_new;
return false;
}
}
There four problems with the implementation of your code:
- It doesn't copy the elements of the list.
- It doesn't assign the value of
new_list
to thelist
variable inmain
- It inserts values from the back to the front, instead of after the last value
max_size
doesn't get updated. It's easy to miss this, because you only increase the size of the array by one each time. That way it would need to allocate each time a value is added. If you increase the new size by more then one it will still reallocate every time.
The first problem can be fixed by changing the for loop in list_add
so it makes a copy:
for (int i = 0; i < space_used; i++) { // this also changed.
list_new[i] = list[i];
cout ...
}
// insert the new value (in the front?)
list_new[max_size-space_used-1] = value;
delete [] list; // Delete the list afterwards instead of earlier.
The second problem can by fixed by returning a pointer to the list. Change the main
function to this:
for (int i = 0; i < 11; i++) {
list = list_add(list, used, N, i);
}
The third problem can be fixed by changing this line
list_new[max_size-space_used-1] = value;
to
list_new[space_used++] = value;
You should also remove the space_used++
after this.
To see the fourth problem you should change this line
int new_max_size = space_used+1;
to
int new_max_size = space_used+3;
It will still reallocate every time. It should however reallocate only two times.
This is the full code:
#include <iostream>
using std::cout;
using std::endl;
int* list_add(int *list, int& space_used, int& max_size, int value) {
if (max_size - space_used > 0) {
list[space_used++] = value;
return list;
}
else {
cout << "Increasing size of array!" << endl;
int new_max_size = space_used+1;
int *list_new = new int[new_max_size];
for (int i = 0; i < space_used; i++) {
list_new[i] = list[i];
cout << list_new[i] << ". ";
}
cout << endl;
list_new[space_used++] = value;
max_size=new_max_size;
delete [] list;
return list_new;
}
}
int main() {
int N = 7;
//declaring dynamic array allocation
int* list = new int[N];
int used = 0, a_val;
for (int i = 0; i < 11; i++) {
list=list_add(list, used, N, i);
}
cout << endl << "Storlek: " << N << endl << endl;
cout << "Printar listan " << endl;
for (int i = 0; i < used; i++) {
cout << list[i] << ". ";
}
}
One problem that jumps out at me is that you're not changing the value of your list pointer outside of the scope of your list_add function. You should make some changes like...
bool list_add(int *list, int& space_used, int max_size, int value)
becomes
bool list_add(int **list, int& space_used, int max_size, int value)
and
list = list_new
becomes
*list = list_new
Otherwise I think you'll find that when you reallocate your list, after returning from list_add your list pointer will still point to the old location.
You have the right idea, but the implementation could use a little 'elbow-grease'
try this:
keep 2 int
capacity - length to which you allocate
size - current end of array
if capacity <= size:
make new list( size = capacity x 2 )
memcopy old list into new list -> if you can't memcopy, copy over the data one-by-one
delete old list
if capacity > size:
list[size] = value
size++
http://www.cplusplus.com/reference/clibrary/cstring/memcpy/
I would worry about this line:
*(list+(max_size-space_used-1)) = value;
and this one:
list_new[i] = i;
There is a lot to be said for knowing how to solve problems, but this isn't one of them.
#include <vector>
#include <iostream>
int main()
{
std::vector<int> numbers;
for (int i = 0; i < 11; i++) {
numbers.push_back(i);
}
for (int i = 0; i < numbers.size(); i++) {
std::cout << numbers[i] << ". ";
}
std::cout << "\n";
}
UPDATE: As shown above in my other answer his function contains at least four bugs in 16 lines. That is a bug for every four lines of code. And then there are the problems with the design of the code. For example the size of the array and the array itself should be together. You can't otherwise guarantee that the function works.
Two of the problems in the code (2,4) could be solved by using a struct
containing the array pointer and the max_size of the data structure. That way you have to pass the two variables together.
精彩评论