• We’re currently investigating an issue related to the forum theme and styling that is impacting page layout and visual formatting. The problem has been identified, and we are actively working on a resolution. There is no impact to user data or functionality, this is strictly a front-end display issue. We’ll post an update once the fix has been deployed. Thanks for your patience while we get this sorted.

declaring an array of pointers in c++

IKeelU

Member
Here's what I declared:

Transaction* tran[10];

Where class Transaction has a default constructor that initializes all member attributes to 0, and constructor that takes in specific values. I compile the program just fine using Borland, but it crashes at run-time without giving me any error message. I have a feeling this has something to do with the default constructor initializing unused values in the heap, but I don't know how to deal with it (nor do I know how to debug the program to make sure this is the problem). What I do know is that when I don't used pointers my program runs fine.

Another thing: Later on in my program, I pass individual elements of that array to a server function that allocates memory for a new Transaction. Basically, I want any changes I make to the pointer in the server function to affect the pointer in my main(). Here's an small portion of my code:

void inputData(Transaction *(*tran))
{
*tran = new Transaction(a, b, c);
}

main()
{
...
inputData(&tran[1]);
...
}

I tried passing the pointer by reference but ended up with a compile error. My only choice therefore was to pass it by pointer. Is there an easier/better way to do this?

In case you're asking, I'm doing this so that memory is allocated as-needed (instead of declaring an array of 10 Transaction elements in the stack form the get-go). More specifically, though, I'm doing this because my prof asked me to do it, and it's due at 8:00 pm tonight. So I would really appreciate any help.


Edit: I removed the default constructor and it still crashes.
 
The code snippet you posted looks fine, so the problem seems to be somewhere else. Post the whole program (preferably a link to a hosted file since the forum doesn't retain formatting).

Also, I recommend using std::vector instead.
 
Originally posted by: IKeelU
void inputData(Transaction *(*tran))
Never seen the syntax done quite like this before, not sure if it will even compile, and if it does it may be treating the extra parens as a strange typecast operation.
What I think you mean is:
void inputData(Transaction **tran)


Also this line:
inputData(&tran[1]);
is probably clearer as "inputData(tran+1);"

My guess would be that the problem has something to do with the constructor itself and that the problem doesn't really have to do with using pointers, they just bring it to the surface more easily.
 
OK, I just found out the deadline was postponed to next Monday 🙂

I also found a better way to pass my pointer (I confirmed this with a classmate of mine):

void inputData(Transaction *(&tran)) {}

main()
{
inputData(tran);
}

This is the same syntax for a normal object only I add the * to tell the function it's a pointer.

I don't know where to post my file, so here's my code including class definitions and implementations.

const double interestRatePerYear = 18.0; //Constants used by all functions
const int sizeName = 20;

class Transaction
{
private:
long accnum;
long trannum;
double amount;

public:

Transaction(long a, long t, double am) //initializes object attributes
{
accnum = a;
trannum = t;
amount = am;
}

double getAmount() const
{ //function retrieves amount involved in transaction
return amount;
}

long getAccNum() const //retrieves account number from transaction element
{
return accnum;
}
};

class Account
{
private:
long num;
double balance;
int count;
char name[sizeName];
double interest;

public:
void initAcct(long n, const char name[]);
void addData(Transaction *(&t));
double chargeInterest();
bool sameAccount(Transaction *(&t)) const;
bool isActive() const;
void printAccount() const;
};

void Account::initAcct(long n, const char person[]) //initializes elements in an account data type
{
num = n;
balance = 0;
count = 0;
for (int i=0;i < sizeName;i++)
name = person;
interest = 0;
}

void Account::addData(Transaction *(&t)) //adds transaction amount to balance in account data type
{
balance = balance + t->getAmount();
count++; //cout<<"5"<<endl;
}

double Account::chargeInterest() //charges interest based on balance and returns interest for that object
{
interest = interest + (interestRatePerYear/100/12*balance);
return interest;
}

bool Account::sameAccount(Transaction *(&t)) const //verifies if account number present in transaction data type matches account
{ //number present in account data type. Will return true if so
if (num == t->getAccNum())
return true;
else
return false;
}

bool Account::isActive() const //returns true if account data type has at least one transaction
{
if (count > 0)
return true;
else
return false;
}

void Account:😛rintAccount() const //prints account info
{

cout<<resetiosflags(ios::right)<<setiosflags(ios::left)<<setw(20)<<name;
cout<<setiosflags(ios::showpoint | ios::fixed | ios::right)
<<setprecision(2)<<setw(9)<<num<<setw(17)<<balance<<setw(12)<<count<<endl;
}

double chargeInterest(Account[], const int&);
void printTable(const Account[], const int);
bool inputData(Transaction*&);
void deleteTransactions(Transaction *[], const int&);


void main()
{
const int max_trans = 10;
const int max_accts = 6;

Transaction* tran[max_trans];
Account accts[max_accts];

accts[0].initAcct(40201, "White"); //initialization of account array elements
accts[1].initAcct(40202, "Orange");
accts[2].initAcct(40203, "Pink");
accts[3].initAcct(40204, "Black");

int num_trans = 0, num_accts = 4;

do
{

if (num_trans > max_trans-1) //breaks loop if number of transactions exceeds "max_trans" (10 in this case)
{
cout<<"Out of memory, input terminated \n \n";
break;
}

if (inputData(tran[num_trans]) == false) //passes Transaction data type to function inputData
break; //breaks loop if user inputs -1
bool flag;
flag = 0;

for (int i=0; i < num_accts; i++) //if account input by user matches an actual account, data will be added to
{ //account data type. Otherwise the program will disregard the data
if (accts.sameAccount(tran[num_trans]))
{
accts.addData(tran[num_trans]);
num_trans++;
flag = 1;
}

else if (i == (num_accts - 1) && flag == 0)
cout<<"Incorrect account: data discarded \n \n";
}
}
while (true);

printTable(accts,num_accts); //prints table containing account info

cout<<"\n\nTotal interest: "<<chargeInterest(accts, num_accts)<<endl; //prints total interest

deleteTransactions(tran, num_trans);
}


double chargeInterest(Account accts[], const int& size) //calculates total interest charged
{
double total = 0;
for(int i=0; i<size; i++)
total = total + accts.chargeInterest();
return total;
}


void printTable(const Account accts[], const int num_accts) //prints table headings, and loops printAccount function
{
int count = 0;

for (int i=0; i<num_accts; i++)
if (accts.isActive() == true)
count++;

cout<<count<<" account(s) were processed \n \n";

if (count == 0)
return;

cout<<setiosflags(ios::left)<<setw(20)<<"Owner"
<<setw(20)<<"Account Number"<<setw(11)<<"Amount"<<setw(20)<<"Num of trans"<<endl<<endl;

for (int i=0; i<num_accts; i++)
{
if (accts.isActive())
accts.printAccount();
}
}



bool inputData(Transaction *(&tran)) //prompts user to input account no. transaction no. and amount
{
long acc, number;
double amount;
cout<<"Enter account number (or -1 to end input): ";
cin>>acc;
if (acc == -1) //if user inputs -1, break loop
{
cout<<"End of input \n \n";
return false;
}
cout<<"Enter transaction number and amount: ";
cin>>number>>amount;
tran = new Transaction(acc, number, amount);
//initializing temp using normal constructor
return true;
}

void deleteTransactions(Transaction *tran[], const int& valid_trans)
{
for (int i=0;i > valid_trans; i++)
delete tran;
}
 
Originally posted by: IKeelU
Transaction* tran[10];

void inputData(Transaction *(*tran)) // This is usually simply written as "**tran" not "*(*tran)"
{
*tran = new Transaction(a, b, c);
}

main()
{
...
inputData(&tran[1]); // This call may actually need to be done as: inputData(&(tran[1]));
...
}

I tried passing the pointer by reference but ended up with a compile error. My only choice therefore was to pass it by pointer. Is there an easier/better way to do this?

Yes, there's a way to pass a pointer by reference (something you tried unsuccessfully). I've done precisely what you're trying to accomplish plenty of times, so I know for a fact that it can be done. Using your sample code above, this is how it I'd re-write it:

void inputData(Transaction *&tran) // Notice the syntax for passing a REFERENCE to a POINTER
{
tran = new Transaction(a, b, c);
}

main()
{
Transaction *tranList[10] = {NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL}; // Always reset your pointers

...
// Now you can pass an individual element of tranList w/o de-referencing it
// Furthermore, tranList[1] will point to a newly constructed object after the call below
inputData(tranList[1]);
...
}


*EDIT* I started composing the above before IKeelU's response above mine. (I got sidetracked in the middle of the post for 20 mins and came back to it).
 
Originally posted by: IKeelU
OK, I just found out the deadline was postponed to next Monday 🙂

I also found a better way to pass my pointer (I confirmed this with a classmate of mine):
... <snip>
I don't know where to post my file, so here's my code including class definitions and implementations.
... <snip>

What exactly are you seeking at this point? Is the above working for you or not?


 
What exactly are you seeking at this point? Is the above working for you or not?

Despite properly referencing the pointers, and resetting my pointers (as TheDiggler suggested), the compiled program still crashes in windows, and gives me a segmentation fault in UNIX. The error/crash occurs after I input my values (the code for which is in inputData, the second to last function).
 
Let's verfiy that it's crashing in inputData by re-writing it as follows:

//prompts user to input account no. transaction no. and amount
bool inputData(Transaction *&tran)
{
cout << "[DEBUG]: Entering inputData(Transaction *& )" << endl;

bool inputResult=true;
long acc, number;
double amount;

cout<<"Enter account number (or -1 to end input): ";
cin>>acc;

//if user inputs -1, break loop
if (acc == -1)
{
cout<<"End of input \n \n";
inputResult=false;
}
else
{
cout << "Enter transaction number and amount: ";
cin >> number >> amount;

cout << "[DEBUG]: Constructing a new Transaction(long,long,double) object ... ";
tran = new Transaction(acc, number, amount);
cout << "<DONE>" << endl;
}

cout << "[DEBUG]: Exiting inputData(Transaction *& ), result: " << inputResult << endl;

return inputResult;
}

Note: This code is best read if you hit QUOTE (to reply to this message w/ quoting it) and then looking at it in the COMPOSE MESSAGE WINDOW(where tabs and spaces expand out).

Now when executing your code, when it enters the inputData() function, you should see:

[DEBUG]: Entering inputData(Transaction *& )
Enter account number (or -1 to end input): <enter a legit value other than -1>
Enter transaction number and amount: <enter legit values>
[DEBUG]: Constructing a new Transaction(long,long,double) object ... <DONE>
[DEBUG]: Exiting inputData(Transaction *& ), result: 0|1


If you see this as your output:

[DEBUG]: Entering inputData(Transaction *& )
Enter account number (or -1 to end input): <enter a legit value other than -1>
Enter transaction number and amount: <enter legit values>
[DEBUG]: Constructing a new Transaction(long,long,double) object ... <Program CRASHES at this point>


then I'll need to think about this some more. Add the above debug code and report the output.
 
This particular function looks wrong:

Originally posted by: IKeelU

void deleteTransactions(Transaction *tran[], const int& valid_trans)
{
for (int i=0;i > valid_trans; i++)
delete tran[I];
}

First off, the loop condition is backwards - it should be i < valid_trans not i > valid_trans
Secondly the messageboard is eating the [ I ] making it look worse than it is 😉 (what first caught my attention).
Third this might cause a crash if the transaction pointers aren't all initialized to NULL when they aren't in use (and valid_trans is being passed in as say 10 rather than the number in use -- assuming the </> mixup was just a typo in the post). One way you can set all the pointers to NULL in one shot is by using memset(tran,0,sizeof(tran)) during the program startup. (Not the case if your main() is as above).
Fourth there is no need to pass an integer by constant reference. The underlying pointer that uses is as big or bigger than an integer, and the extra indirection just decreases efficiency and clarity.
 
I implemented all the changes glugglug suggested. I also ran the program with the various debug statements TheDiggler wrote, and the crash occurs after all the statements have been printed. The program doesn't always crash after the first "tran" element is requested. I've actually managed to input 8 transactions before a crash. Note that the crashes has never occured when I input -1 for the account number (which causes the program to exit the main loop, run the destructor then quit).

Hope this helps narrow down the problem.
 

Right after my last post I made some changes. Instead of passing the array element, I passed a temporary Transaction element to inputData, then back in main() I pointed the first array element to the contents of the temp. I ran it twice without crashes, so I think this solved the problem (I honestly don't know why). Here's the revised code for the main loop:

do
{
Transaction* tran_temp = NULL;
if (num_trans > max_trans-1)
{
cout<<"Out of memory, input terminated \n \n";
break;
}

if (inputData(tran_temp) == false)
break;

bool flag;
flag = 0;

for (int i=0; i < num_accts; i++)
{
if (accts.sameAccount(tran_temp))
{
accts.addData(tran_temp);
tran[num_trans] = new Transaction;
tran[num_trans] = tran_temp;
delete tran_temp;
num_trans++;
flag = 1;
}

else if (i == (num_accts - 1) && flag == 0)
{
cout<<"Incorrect account: data discarded \n \n";
}
}
}

I also added a default constructor to class Transaction.
Anyway, thanks everyone for taking the time to read my code and point out errors - it's more than my TA's did.
 
Originally posted by: IKeelU

if (accts[i].sameAccount(tran_temp))
{
accts[i].addData(tran_temp);
tran[num_trans] = new Transaction; // Step 1 (See comments below)
tran[num_trans] = tran_temp; // Step 2 (See comments below)
delete tran_temp; // Step 3 (See comments below)
num_trans++;
flag = 1;
}

If your prof looks at the logic above, he'll shoot you (or at least give you a poor grade) 😛.

Step 1: You're assigning tran[num_trans] to a new Transaction Object (invoking the default constructor)
Step 2: You're now pointing tran[num_trans] to the location of tran_temp
Step 3: Now you're deleting the memory allocated by tran_temp

After step 3, the pointer in Step 2 is pointing to DEALLOCATED MEMORY! I don't know how/why your program is working; however, that logic you've posted is dangerous. Furthermore, the new object you create in Step 1 becomes a MEMORY LEAK as the pointer which was pointing to that object immediately gets pointed to a different object (tran_temp).

Since you've made other changes to your program, what don't you re-post your latest incarnation for further assistance.

*EDIT* Forgot to mention, if your program crashes AFTER IT HAS EXITED THE inputData() function, then the bug is not in inputData(). Add similar such "Entering/Exiting" DEBUG STATEMENTS to every FUNCTION & METHOD you've defined and you'll quickly narrow down which function/method is the culprit.
 
Out of curiosity, what is the output if you add the following lines before the input loop:

cout << "Address of tran: " <<(long)tran << "\n";
cout << "Address of accts: " << (long)accts << "\n";
 
Step 3: Now you're deleting the memory allocated by tran_temp

:Q Don't know what I was thinking. I deleted the line and the program works fine (just like it did before I deleted it - you'll see my program doesn't actually use the Transaction database for anything [yet]).

Out of curiosity, what is the output if you add the following lines before the input loop:
cout << "Address of tran: " <<(long)tran << "\n";
cout << "Address of accts: " << (long)accts << "\n";

Here's what gets printed out when I add the lines glugglug requested:

Address of tran: 1244976
Address of accts: 1244688

Like I just said, my program no longer crashes, but The Diggler requested that I show my code in its entirety, so here it is:



#include <iostream.h>
#include <iomanip.h>
using namespace std;

const double interestRatePerYear = 18.0;
const int sizeName = 20;

class Transaction
{
private:
long accnum;
long trannum;
double amount;

public:

Transaction()
{
accnum = 0;
trannum = 0;
amount = 0;
}
Transaction(long a, long t, double am)
{
accnum = a;
trannum = t;
amount = am;
}

double getAmount() const
{
return amount;
}

long getAccNum() const
{
return accnum;
}
};

class Account
{
private:
long num;
double balance;
int count;
char name[sizeName];
double interest;

public:
void initAcct(long n, const char name[]);
void addData(Transaction *(&t));
double chargeInterest();
bool sameAccount(Transaction *(&t)) const;
bool isActive() const;
void printAccount() const;
};

void Account::initAcct(long n, const char person[])
{
num = n;
balance = 0;
count = 0;
for (int i=0;i < sizeName;i++)
name = person;
interest = 0;
}

void Account::addData(Transaction *(&t))
{
balance = balance + t->getAmount();
count++; //cout<<"5"<<endl;
}

double Account::chargeInterest()
{
interest = interest + (interestRatePerYear/100/12*balance);
return interest;
}

bool Account::sameAccount(Transaction *(&t)) const
{
if (num == t->getAccNum())
return true;
else
return false;
}

bool Account::isActive() const
{
if (count > 0)
return true;
else
return false;
}

void Account:😛rintAccount() const
{

cout<<resetiosflags(ios::right)<<setiosflags(ios::left)<<setw(20)<<name;
cout<<setiosflags(ios::showpoint | ios::fixed | ios::right)
<<setprecision(2)<<setw(9)<<num<<setw(17)<<balance<<setw(12)<<count<<endl;
}

double chargeInterest(Account[], const int&);
void printTable(const Account[], const int);
bool inputData(Transaction*&);
void deleteTransactions(Transaction *[], const int&);


void main()
{
const int max_trans = 10;
const int max_accts = 6;

Transaction* tran[max_trans];
memset(tran,0,sizeof(tran));
Account accts[max_accts];


accts[0].initAcct(40201, "White");
accts[1].initAcct(40202, "Orange");
accts[2].initAcct(40203, "Pink");
accts[3].initAcct(40204, "Black");

int num_trans = 0, num_accts = 4;

do
{
Transaction* tran_temp = NULL;
if (num_trans > max_trans-1)
{
cout<<"Out of memory, input terminated \n \n";
break;
}

if (inputData(tran_temp) == false)
break;

bool flag;
flag = 0;

for (int i=0; i < num_accts; i++)
{
if (accts.sameAccount(tran_temp))
{
accts.addData(tran_temp);
tran[num_trans] = new Transaction;
tran[num_trans] = tran_temp;

num_trans++;
flag = 1;
}

else if (i == (num_accts - 1) && flag == 0)
cout<<"Incorrect account: data discarded \n \n";
}
}
while (true);

printTable(accts,num_accts);

cout<<"\n\nTotal interest: "<<chargeInterest(accts, num_accts)<<endl;

deleteTransactions(tran, num_trans);
}


double chargeInterest(Account accts[], const int& size)
{
double total = 0;
for(int i=0; i<size; i++)
total = total + accts.chargeInterest();
return total;
}


void printTable(const Account accts[], const int num_accts)
{
int count = 0;

for (int i=0; i<num_accts; i++)
if (accts.isActive() == true)
count++;

cout<<count<<" account(s) were processed \n \n";

if (count == 0)
return;

cout<<setiosflags(ios::left)<<setw(20)<<"Owner"
<<setw(20)<<"Account Number"<<setw(11)<<"Amount"<<setw(20)<<"Num of trans"<<endl<<endl;

for (int i=0; i<num_accts; i++)
{
if (accts.isActive())
accts.printAccount();
}
}



bool inputData(Transaction *(&tran))
{
long acc, number;
double amount;
cout<<"Enter account number (or -1 to end input): ";
cin>>acc;
if (acc == -1)
{
cout<<"End of input \n \n";
return false;
}

cout<<"Enter transaction number and amount: ";
cin>>number>>amount;
tran = new Transaction(acc, number, amount);

return true;
}

void deleteTransactions(Transaction *tran[], const int& valid_trans)
{
for (int i=0;i <= valid_trans; i++)
delete tran;
}
 
Originally posted by: IKeelU
Step 3: Now you're deleting the memory allocated by tran_temp

Here's what gets printed out when I add the lines glugglug requested:

Address of tran: 1244976
Address of accts: 1244688

Odd, tran is positioned with an address 288 bytes higher than accts, intuitively I expected the opposite. This works out to exactly 48 bytes allocated per account assuming they are adjacent parts of memory, which is reasonable (24 for the ints, longs and doubles assuming int and long are both 4 bytes, then 20 for the name, plus 4 byte padding apparently). I was thinking maybe tran wasn't being interpreted as the type we think it is, but this doesn't tell us this since tran has the higher memory location, unless maybe if you reverse the order they are declared...

This line is still a memory leak (and is totally unnecessary) as thediggler pointed out before:

tran[num_trans] = new Transaction; (the one right before tran[num_trans] = tran_temp)
 
This line is still a memory leak (and is totally unnecessary) as thediggler pointed out before:

tran[num_trans] = new Transaction; (the one right before tran[num_trans] = tran_temp)

Thanks.

When I pass my Transaction array element into inputData (i.e. if I don't use a temporary Transaction element), it never crashes if I input the last account element (there are four pre-defined elements in my account array). As soon I use elements 0, 1, or 2 of my account array as input, I get a crash.😕
 
I haven't yet had a chance to look at the latest changes; however, ikeelu e-mailed me a copy of his source code so that I could throw it up on a web server. To see a CLEANER/FORMATTED version of his code (not sure if it's the very latest), go to:

www.realmud.com/ikeelu/q3.cpp.txt

Note: I added a ".txt" file extension to his filename so that the file will display "as-is" in a web browser.

*EDIT Link fixed
 
Originally posted by: IKeelU
This line is still a memory leak (and is totally unnecessary) as thediggler pointed out before:

tran[num_trans] = new Transaction; (the one right before tran[num_trans] = tran_temp)

Thanks.

When I pass my Transaction array element into inputData (i.e. if I don't use a temporary Transaction element), it never crashes if I input the last account element (there are four pre-defined elements in my account array). As soon I use elements 0, 1, or 2 of my account array as input, I get a crash.😕

Then that means somewhere in your code you're over-writing existing memory. As I said in my previous post, I'll take a closer look at this when I have a bit more time. In the meantime, look more closely/carefully at the memory you're allocating.
 
Ok, take a look at the following update:

www.realmud.com/ikeelu/q3_dig1.cpp.txt

Note 1: I reformatted the code to my preference (it made it easier for me to go through your code)
Note 2: There are plenty of comment boxes detailing the changes
Note 3: I haven't compiled or tested my changes. There may be typos in the code.
 
The revised code crashed during "deleteTransactions". I took care of it by setting the loop condition to "i < valid_trans" instead "i <= valid_trans". Anyway, the program is now finally working as intended.

Thanks again.

btw, I noticed my crashes stopped as soon as I implemented the "sanity checks" (i.e. if (t != NULL)) inside my Account member functions. I wonder, how could this have helped? Any transaction I passed to those functions couldn't have been "NULL" in the first place, so those statements shouldn't have changed anything.
 
I agree that the addition of the sanity check "if (t != NULL)" should not have been necessary. While you said your program "works," does it output the correct information?

If you could post your final program on the FTP site to take a quick peek as to why the sanity check may have been necessary, that may help solve this final mystery. I'm glad to hear though that you're basically done w/ this assignment. 😉
 
Here's the new [final] version

q3_v2.txt

and my output is good btw.


I agree that the addition of the sanity check "if (t != NULL)" should not have been necessary.

Hmmm...I'll ask my teacher if she knows why my program wouldn't work without them. I don't recall her ever telling us to use them, so I must have done something else wrong. Either way, I've already submitted it (the deadline is 12 hours away, and I need sleep).
 
Since you already turned in your assignment there's not much you can do about the following this time; however, it's good info for you to have:

In main(), you have a blurb at the end:
for (int i=0; i < num_trans; i++) tran[i] -> deleteTransaction();

where that's invoking:
Transaction::DeleteTransaction() { delete *this; }

It's VERY dangerous to have an object delete itself. I realize you probably implemented this when you kept crashing in the cleanup stage (due to the i<=num_trans bug instead of i<num_trans); however, you got lucky this time that the approach you took didn't bite you in the A$$. Don't be surprised if your instructor deducts points for this.

The correct way to delete the objects is to do what you originally did:
for (int i=0; i < num_trans; i++) delete tran[i]; // Even safer: wrap it with an "if (tran[i] != NULL)" sanity check

P.S. Nice job overall on the assignment. It's much cleaner than what you started out with.
 
Back
Top