Reputation: 5
I tried solving this problem: http://projecteuler.net/problem=201 But the program below breaks for larger SET size(>20) throwing a stack overflow exception. Is there a memory leak happening? Could you please point out where? Also if the following implementation involves bad coding practice, please let me know. I'm trying to improve my amateur programming skills. Thanks in advance. EDIT: I edited the code below. I don't get any stack overflow exceptions now. But could anyone suggest me a better algorithm? Thanks!
#include "stdafx.h"
#include <iostream>
#include <cmath>
#include <winsock2.h>
#define SET 100
#define SUBSET 50
class node
{
public:
int data;
node* next;
};
using namespace std;
int sum(int A[SUBSET]);
void combCalc(int S[SET]);
bool search(node* head, int sum);
void appendIfUnique(int total, bool nullHeader);
int _tmain(int argc, _TCHAR* argv[])
{
cout << "Hello World" << endl;
int S [SET];
for(int i=1; i<=SET; i++)
{
S[i-1]= (int)pow((float)i,2);
}
combCalc(S);
cin.get();
return 0;
}
static node *head;
static node *current = head;
void combCalc(int S[])
{
int row=0, col=0;
int elePos[SUBSET], B[SUBSET];
bool nullHeader = true;
for(int z=0; z<SUBSET; z++) // initializing elePos
{
elePos[z]=z;
}
bool notDone = true;
while (notDone || col <(SUBSET-1))
{B[col] = S[elePos[col]];
if(col==(SUBSET-1)) //finished forming a subset
{
notDone = false;
for(int q=(SUBSET-1); q>=0; q--) //incrementing from the last element
{
if(elePos[q]<(SET-SUBSET+q)) //checking if the element has reached its maximum
{
notDone = true;
elePos[q]++;
for(int w=q+1; w<SUBSET; w++) //setting the trailing elements to its minimum
{
elePos[w]=elePos[q]+w-q;
}
break;
}
}
if(notDone){col=0;row++;}
int total = sum(B);
appendIfUnique(total,nullHeader);
nullHeader = false;
}
else
{
col++;
}
}
int result = 0;
for(node *pnode = head; pnode != current->next; pnode=pnode->next)
result = result + pnode->data;
cout << result << endl;
}
int sum(int A[])
{
int total = 0;
for(int i=0; i<SUBSET; i++)
{
total = total + A[i];
}
return total;
}
bool search(node* head, int sum)
{
bool exists = false;
node* pNode = head;
while(pNode != NULL)
{
if(pNode->data == sum)
{
exists = true;
break;
}
pNode = pNode->next;
}
return exists;
}
void appendIfUnique(int total, bool nullHeader)
{
if(nullHeader){head = NULL;}
if(!search(head,total))
{
node *temp;
/*temp=(node *) malloc(sizeof(node));*/
temp = new node();
temp->data = total;
temp->next = NULL;
if(head == NULL)
{
head = current = temp;
}
else
{
current->next = temp;
current = temp;
}
}
}
Upvotes: 0
Views: 222
Reputation: 5102
Some notes:
combCalc(S)
(in my case, it dies after 32K calls).As it has been indicated in the comments, you should probably reconsider your algorithm. In the meantime, a simple modification is to remove the recursion (since it is not even a proper recursion):
int _tmain(int argc, _TCHAR* argv[])
{
cout << "Hello World" << endl;
int S [SET];
for(int i=1; i<=SET; i++)
{
S[i-1]= (int)pow((float)i,2);
}
while(combCalc(S)) { } // <--- keep calling while combCalc is true
cin.get();
return 0;
}
by making combCal() return a bool:
bool combCalc(int S[]) // <--- MODIFY (check also the forward declaration)
{
...
if(notDone || col <(SUBSET-1))
{
...
return true; // <--- MODIFY return true... I need to keep calculating.
}
else
{
int result = 0;
for(node *pnode = head; pnode != current->next; pnode=pnode->next)
result = result + pnode->data;
cout << result << endl;
return false; // <--- MODIFY return false.... we're done
}
}
The above is just a minimum modification. I'm not even sure it solves the problem correctly since I haven't really looked at the algorithm.
You should consider:
#define
for constants.malloc
but using new
(more C++)free
this means that the memory you allocated will not be released. (use delete
if you're using new
or delete []
if you did new []
)Upvotes: 1