HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Dynamically-sized stack - follow-up 2

Submitted by: @import:stackexchange-codereview··
0
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:

  • 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 capacityIncrement variable 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 by capacityIncrement



```
#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.

// 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.