constructArray
doesn’t actually do anything relevant, set aside leaking memory. Same for printArray
.
I guess you began to code in c++ not that long ago, the code you’ve written does the following:
void constructArray (No *Number, int size) {
// allocates enough memory to store n=size pointers to No then constructs them
// stores the address of that memory in temp
No **temp = new No *[size];
// for each pointer to No *temp, *(temp+1), ..., *(temp+size-1)
for (int i = 0; i < size; i++)
{
// allocates enough space to store a No, constructs a No, and assigns
// its address to *(temp+i)
temp[i] = new No;
// initializes the values of the newly allocated No
temp[i]->decimal = rand() % 1000;
temp[i]->binary = "0";
temp[i]->octal = "0";
temp[i]->hexadecimal = "0";
}
// discards temp, effectively leaking the memory allocated in this function
}
void printArray (No *Number, int size) {
// prints a string
cout << "Decimal\t" << "Binary\t\t\t" << "Octal\t\t" << "Hexadecimal" << endl;
// allocates enough memory to store n=size pointers to No then constructs them
// stores the address of that memory in temp
No **temp = new No *[size];
// for each pointer to No *temp, *(temp+1), ..., *(temp+size-1)
for (int i = 0; i < size; i++) {
// allocates enough space to store a No, constructs a No, and assigns
// its address to *(temp+i)
temp[i] = new No;
// prints the content of the newly allocated No
cout << temp[i]->decimal << "\t"
<< temp[i]->binary << "\t\t\t"
<< temp[i]->octal << "\t\t"
<< temp[i]->hexadecimal << endl;
}
// discards temp, effectively leaking the memory allocated in this function
}
There are several oddities and many things wrong with your code:
-
as melpomene stated in the comments, you’re not using some parameters of your functions (sometimes, its actual useful to define unused parameters but not in this case)
-
you forget to either return or delete what you have dynamically allocated in your function
-
you make too many dynamic allocations (in particular in printArray where no dynamic allocation should be needed)
-
you assign string literals to
char*
, you should either store them inconst char*
or manually allocate the memory (a string literal will be stored in the data segment, and the data segment shouldn’t be mutable, that’s why the standard forbids it) some compilers will accept it with a warning though, but it’s not clean -
you use a pointer to pointer to
No
to dynamically allocate an array ofNo
s -
you also uselessly split the strings in
std::cout
calls and delegate the initialization of aNo
toconstructArray
rather than toNo
‘s constructor
I think what you actually want to write is the following:
#include <iostream>
#include <ctime>
using namespace std; // i usually avoid this
struct No {
// No constructor, each time you "construct" a No instance, this function will
// be called (the No() : var(value), ... syntax is called a member
// initializer list, it's specific to constructors)
No() :
decimal(rand() % 1000),
binary("0"),
octal("0"),
hexadecimal("0")
{}
int decimal;
const char *binary;
const char *octal;
const char *hexadecimal;
};
void constructArray (No*& arrayDestination, int size) {
// allocates enough memory for n=size No and constructs them
// then stores the address of this allocated memory in arrayDestination
arrayDestination = new No[size];
// NOTE: arrayDestination is a reference to a pointer to No, which means
// that you'll actually modfiy the parameter used by the caller
// in other words:
//
// No* ptr = nullptr;
// constructArray(ptr, size);
//
// will actually modify ptr, another way would be to return the pointer
// with the return value of constructArray
// no need to initialize the No allocated here, new[] already called the
// constructor
}
void printArray (No* array, int size) {
// no need to split the string here...
cout << "Decimal\tBinary\t\t\tOctal\t\tHexadecimal" << endl;
// don't reallocate an array, the array has always been constructed, why would
// you want another one? (plus an uninitialized one...)
for (int i = 0; i < size; i++) {
// actually use the first parameter!
// NOTE: now, this isn't a reference (No* instead of No*&) because we
// don't need to modify this parameter, the parameter is actually
// copied when we call the function
cout << array[i].decimal << "\t"
<< array[i].binary << "\t\t\t"
<< array[i].octal << "\t\t"
<< array[i].hexadecimal << endl;
}
}
int main() {
// if you use rand, you should call srand first to initialize the random
// number generator (also note that rand is deprecated)
srand(time(nullptr));
// NOTE: time(nullptr) returns the current timestamp
// it's sometimes used as a pseudo-random "seed" to initialize a
// generator
// this will be a pointer to the first element of our "arrray"
No *array = nullptr;
// this instruction is valid and will give you an integer between 1 and 9
int size = (rand() % 9) + 1;
// now we call the corrected functions
constructArray(array, size);
printArray(array, size);
// you should actually release the array once you're finished using it
// the clean way would be to implement a "releaseArray" function,
// but i'm lazy, so i'll just do it here:
delete[] array;
// don't forget main returns an int! 0 is usually the code for "nothing bad happened"
return 0;
}
Even better, replace the dynamically allocated array of No
by a std::vector<No>
and the const char*
by std::string
(if you want them to be mutable). That way you won’t need a function to build/release the array anymore.
Edit: mmh it took me a bit too much time to post that
0
solved How to access array of pointer to structure [closed]