patterncppModerate
Stack challenge - improving memory consumption
Viewed 0 times
stackchallengememoryimprovingconsumption
Problem
I am working on this problem:
Stacks
Imagine, that you are employed by a software development company. You
work now on the famous "D++ project", which is devoted to the creation
of a new generation programming language. Your particular task is
quite prosaic, though. You are to develop the memory manager being
able to work with a large number of stacks.
Input
The first line of the input contains the total number of stack
operations \$N\$, \$0 < N ≤ 100000\$. Each of the next \$N\$ lines contains a
description of a stack operation, either in the form PUSH A B (meaning
to push B into stack A), or in the form POP A (meaning to pop an
element from stack A), where A is the number of stack (\$1 ≤ A ≤ 1000\$),
and B is an integer (\$0 ≤ B ≤ 10^9\$). You may assume, that every
operation is correct (i.e., before each POP operation, the respective
stack is not empty).
Output
For each POP operation, described in the input, output the value,
which this POP operation gets from the top of that stack, to which it
is applied. Numbers should appear according to the order of the POP
operations in the input. Each number should be output in a separate
line.
I have developed a solution here:
```
#include
class MyStack
{
public:
int data;
MyStack *link;
MyStack *top;
MyStack()
{
link = NULL;
top = NULL;
}
void push( int data )
{
MyStack tmp = (MyStack ) malloc( sizeof( MyStack ) );
tmp->data = data;
tmp->link = top;
top = tmp;
}
void pop()
{
MyStack *tmp = top;
top = top->link;
printf( "%d\n", tmp->data );
free( tmp );
}
void clear()
{
while( top != NULL )
{
MyStack *tmp = top;
top = top->link;
free( tmp );
}
}
bool isEmpty()
{
Stacks
Imagine, that you are employed by a software development company. You
work now on the famous "D++ project", which is devoted to the creation
of a new generation programming language. Your particular task is
quite prosaic, though. You are to develop the memory manager being
able to work with a large number of stacks.
Input
The first line of the input contains the total number of stack
operations \$N\$, \$0 < N ≤ 100000\$. Each of the next \$N\$ lines contains a
description of a stack operation, either in the form PUSH A B (meaning
to push B into stack A), or in the form POP A (meaning to pop an
element from stack A), where A is the number of stack (\$1 ≤ A ≤ 1000\$),
and B is an integer (\$0 ≤ B ≤ 10^9\$). You may assume, that every
operation is correct (i.e., before each POP operation, the respective
stack is not empty).
Output
For each POP operation, described in the input, output the value,
which this POP operation gets from the top of that stack, to which it
is applied. Numbers should appear according to the order of the POP
operations in the input. Each number should be output in a separate
line.
I have developed a solution here:
```
#include
class MyStack
{
public:
int data;
MyStack *link;
MyStack *top;
MyStack()
{
link = NULL;
top = NULL;
}
void push( int data )
{
MyStack tmp = (MyStack ) malloc( sizeof( MyStack ) );
tmp->data = data;
tmp->link = top;
top = tmp;
}
void pop()
{
MyStack *tmp = top;
top = top->link;
printf( "%d\n", tmp->data );
free( tmp );
}
void clear()
{
while( top != NULL )
{
MyStack *tmp = top;
top = top->link;
free( tmp );
}
}
bool isEmpty()
{
Solution
My main problem is that you are dynamically allocating all objects with
See no need to call new at all most of the time.
The second problem is that you are using
The third observation is that you are building a container (linked list). There is already a built-in linked list.
For this structure of a stack. The list is not the optimal data structure. A
If you want multiple stacks. I would just create a std::vector of stacks that you can the size at runtime to get all the stacks you need.
That should solve your memory problems.
Looking at your code we could go into your design of MyStack (but I think that is superflous). But some common rules:
Only use
But you should not even do that normally:
a. Use local object most of the time.
b. Use containers when you have groups of objects.
c. When you really need dynamic allocation use smart pointers.
new and or malloc. In C++ object can be created locally without new just by declaring them in the local scope.MyStack stack; // Creates and initializes a MyStack object.
MyStack data[10]; // Create and initialize an array of MyStack objects.See no need to call new at all most of the time.
The second problem is that you are using
malloc. This is C memory management. The blocks are not necessarily in the same areas as memory allocated by new and thus not compatible to be deleted by either. Also they don't run the constructor and free will not run the destructor. So a very bad idea.The third observation is that you are building a container (linked list). There is already a built-in linked list.
std::list data;For this structure of a stack. The list is not the optimal data structure. A
std::vector or std::deque is probably better. But we have a built-in std::stack which is actually a container adapter that wraps another container type (by default it uses std::deque).std::stack data; // This is probably your best option
// for holding data.If you want multiple stacks. I would just create a std::vector of stacks that you can the size at runtime to get all the stacks you need.
std::vector> data;
data.resize();That should solve your memory problems.
Looking at your code we could go into your design of MyStack (but I think that is superflous). But some common rules:
- Don't use C facilities when better C++ ones exist.
scanf() => operator>>- Don't mix dynamic memory allocation.
Only use
new and delete in C++ code.- Every call to
newshould be matched with a call todelete
But you should not even do that normally:
a. Use local object most of the time.
b. Use containers when you have groups of objects.
c. When you really need dynamic allocation use smart pointers.
- Learn the Rule of 3
Code Snippets
MyStack stack; // Creates and initializes a MyStack object.
MyStack data[10]; // Create and initialize an array of MyStack objects.std::list<int> data;std::stack<int> data; // This is probably your best option
// for holding data.std::vector<std::stack<int>> data;
data.resize(<Number-Of-Stacks-You-Need>);Context
StackExchange Code Review Q#54812, answer score: 10
Revisions (0)
No revisions yet.