patterncMinor
Dynamically-sized stack - follow-up 2
Viewed 0 times
sizeddynamicallyfollowstack
Problem
Follow up of - Dynamically-sized stack - follow up
I've took the tips given to me, and what I did now is:
```
#include
#include
#include
typedef struct
{
int *elementData;
unsigned int stackSize;
unsigned int capacityIncrement;
unsigned int elementCount;
} Stack;
void stack_initialize(Stack*, unsigned int);
void stack_push(Stack*, int);
void stack_pop(Stack*);
int stack_peek(const Stack*);
void stack_destroy(Stack*);
bool stack_isEmpty(const Stack*);
void stack_setCapacityIncrement(Stack*, unsigned int);
unsigned int stack_getCapacityIncrement(const Stack*);
unsigned int stack_getNumberOfElements(const Stack*);
unsigned int stack_getSize(const Stack*);
int main()
{
Stack s1;
stack_initialize(&s1, 4);
for(int i = 0; i elementData = NULL;
p->stackSize = 0;
p->capacityIncrement = capacityIncrement;
p->elementCount = 0;
}
void stack_push(Stack *p, int value)
{
if (p->elementCount == p->stackSize)
{
int newStackSize = p->stackSize + p->capacityIncrement;
void temp = realloc(p->elementData, sizeof(p->elementData) * newStackSize);
if (temp == NULL || newStackSize == 0)
{
return;
}
p->stackSize = newStackSize;
p->elementData = temp;
}
p->elementData[p->elementCount] = value;
p->elementCount++;
}
void stack_pop(Stack *p)
{
if (!stack_isEmpty(p))
{
p->elementCount--;
if
I've took the tips given to me, and what I did now is:
- changing the function names to have a prefix
- making the
pop()function only popping and not returning the value as well
- checking for OOM exception / scenario that 0 is passed to
capacityIncrementvariable when initializing the stack (in those cases, if trying to push values to the stack, just ignore)
- added a decrement mechanism, when popping values a check will be made to see if there are (
capacityIncrement / 2 + capacityIncrement) elements in the stack that are not in use, and if so, decrement the stack size bycapacityIncrement
```
#include
#include
#include
typedef struct
{
int *elementData;
unsigned int stackSize;
unsigned int capacityIncrement;
unsigned int elementCount;
} Stack;
void stack_initialize(Stack*, unsigned int);
void stack_push(Stack*, int);
void stack_pop(Stack*);
int stack_peek(const Stack*);
void stack_destroy(Stack*);
bool stack_isEmpty(const Stack*);
void stack_setCapacityIncrement(Stack*, unsigned int);
unsigned int stack_getCapacityIncrement(const Stack*);
unsigned int stack_getNumberOfElements(const Stack*);
unsigned int stack_getSize(const Stack*);
int main()
{
Stack s1;
stack_initialize(&s1, 4);
for(int i = 0; i elementData = NULL;
p->stackSize = 0;
p->capacityIncrement = capacityIncrement;
p->elementCount = 0;
}
void stack_push(Stack *p, int value)
{
if (p->elementCount == p->stackSize)
{
int newStackSize = p->stackSize + p->capacityIncrement;
void temp = realloc(p->elementData, sizeof(p->elementData) * newStackSize);
if (temp == NULL || newStackSize == 0)
{
return;
}
p->stackSize = newStackSize;
p->elementData = temp;
}
p->elementData[p->elementCount] = value;
p->elementCount++;
}
void stack_pop(Stack *p)
{
if (!stack_isEmpty(p))
{
p->elementCount--;
if
Solution
-
Use same case prefix for type "Stack" and functions "stack" --> "gStack" or whatever.
-
Use correct print specifier. This implies that either OP does not have all warnings enabled or is using a weak compiler. Suggest remedying that.
-
Use consistent sign-ness
-
In
-
Silently failing
-
-
Consider putting all memory allocation into one helper function. (BTW:
Use same case prefix for type "Stack" and functions "stack" --> "gStack" or whatever.
-
Use correct print specifier. This implies that either OP does not have all warnings enabled or is using a weak compiler. Suggest remedying that.
// printf("Size of stack is %d.\n", s1.stackSize);
printf("Size of stack is %u.\n", s1.stackSize);-
Use consistent sign-ness
// int newStackSize;
unsigned newStackSize;-
In
push(), newStackSize test should be corrected or eliminated.// if (temp == NULL || newStackSize == 0)
if (temp == NULL && newStackSize > 0)
// or if confident newStackSize will never be 0 here
if (temp == NULL)-
Silently failing
push() due to OOM is a concern.-
pop() should do similar OOM protection as pop(). Even though it sounds silly that reducing memory usage should ever fail.-
Consider putting all memory allocation into one helper function. (BTW:
destroy() did not free p->elementData.static int stack_realloc(int **ptr, unsigned *oldsize, unsigned newsize) {
if (newsize > 0) {
void *newptr = realloc(*ptr, newsize);
if (newptr == NULL) return 1; // fail
*ptr = newptr;
}
else {
free(*ptr);
*ptr = NULL;
}
*oldsize = newsize;
return 0; // success;
}
// push()
unsigned newStackSize = p->stackSize + p->capacityIncrement;
if (stack_realloc(&p->elementData, &p->stackSize, newStackSize);
// pop()
// change from == to >=
if(p->stackSize - p->elementCount >= p->capacityIncrement / 2 + p->capacityIncrement) {
unsigned newStackSize = p->stackSize - p->capacityIncrement;
// No problem is realloc failed, just continue with current stack
stack_realloc(&p->elementData, &p->stackSize, newStackSize);
// destroy()
stack_realloc(&p->elementData, &p->stackSize, 0);
// Do not free `p`.
// free(p);Code Snippets
// printf("Size of stack is %d.\n", s1.stackSize);
printf("Size of stack is %u.\n", s1.stackSize);// int newStackSize;
unsigned newStackSize;// if (temp == NULL || newStackSize == 0)
if (temp == NULL && newStackSize > 0)
// or if confident newStackSize will never be 0 here
if (temp == NULL)static int stack_realloc(int **ptr, unsigned *oldsize, unsigned newsize) {
if (newsize > 0) {
void *newptr = realloc(*ptr, newsize);
if (newptr == NULL) return 1; // fail
*ptr = newptr;
}
else {
free(*ptr);
*ptr = NULL;
}
*oldsize = newsize;
return 0; // success;
}
// push()
unsigned newStackSize = p->stackSize + p->capacityIncrement;
if (stack_realloc(&p->elementData, &p->stackSize, newStackSize);
// pop()
// change from == to >=
if(p->stackSize - p->elementCount >= p->capacityIncrement / 2 + p->capacityIncrement) {
unsigned newStackSize = p->stackSize - p->capacityIncrement;
// No problem is realloc failed, just continue with current stack
stack_realloc(&p->elementData, &p->stackSize, newStackSize);
// destroy()
stack_realloc(&p->elementData, &p->stackSize, 0);
// Do not free `p`.
// free(p);Context
StackExchange Code Review Q#62694, answer score: 3
Revisions (0)
No revisions yet.