code review request: stack implementation

This is a discussion on code review request: stack implementation within the C Programming forums, part of the General Programming Boards category; I'm looking for some input on my stack implementation. Anything goes -- style, structure, functionality. Anything I should have done ...

  1. #1
    Registered User
    Join Date
    Jan 2008
    Posts
    69

    code review request: stack implementation

    I'm looking for some input on my stack implementation. Anything goes -- style, structure, functionality. Anything I should have done differently?

    stack.h
    Code:
    #include <stdio.h>
    
    typedef int stackElementT;
    
    typedef struct {
    
      stackElementT *contents;
      int size;
      int count;
    
    } stackT;
    
    void STACKinit(stackT *stack, unsigned int);
    void STACKpush(stackT *stack, int value);
    int STACKpop(stackT *stack);
    void STACKempty(stackT *stack);
    stack.c

    Code:
    #include <stdio.h>                                               
    #include <stdlib.h>                                              
    #include "stack.h"                                               
    
    void                                                             
    STACKinit(stackT *stack, unsigned int size) {
      
      stack->contents = malloc(sizeof(stackElementT) * size);        
      stack->size = size;                                            
      stack->count = 0;                                              
                                                                     
    }
    
    void                                                             
    STACKpush(stackT *stack, const stackElementT value) {            
                                                                     
      if (stack->count >= stack->size) {
        
        fprintf(stderr, "Cannot push onto a full stack.\n");         
        exit(1);                                                     
                                                                     
      }
                                                                     
      stack->contents[stack->count++] = value;                       
                                                                     
    }
    
    stackElementT                                                    
    STACKpop(stackT *stack) {
                                                                     
      if (stack->count <= 0) {
        
        fprintf(stderr, "Cannot pop off empty stack.\n");            
        exit(1);                                                     
                                                                     
      }
                                                                     
      return stack->contents[--stack->count];                        
                                                                     
    }
    
    void 
    STACKempty(stackT *stack) {                                      
      STACKinit(stack, stack->size);                                 
    }

  2. #2
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    I do not see why your header file needs
    #include <stdio.h>

    you should check the malloc return
    exit is too harsh way of notifying the caller that he did something wrong - better return error code

    You do not provide function that will free allocated memory
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  3. #3
    Registered User
    Join Date
    Jan 2008
    Posts
    69
    Quote Originally Posted by vart View Post
    exit is too harsh way of notifying the caller that he did something wrong - better return error code
    How would you do this with functions that already have a return type, i.e. STACKpop?

    You do not provide function that will free allocated memory
    How is this typically done?

  4. #4
    Woof, woof! zacs7's Avatar
    Join Date
    Mar 2007
    Location
    Australia
    Posts
    3,459
    A few things, some personal

    * the function names are a little 'odd' and hard to read. stackPop() or stack_pop() would be nicer to use IMO.
    * --stack->count I'd use parenthesis to avoid operator precedence... just me --(stack->count). Same for stack->count++ -- I find (stack->count)++ easier to read.
    * use inclusion guards in stack.h
    * no free()?
    * STACKinit(stackT *stack, unsigned int size)... consider using 'size_t size' instead of 'unsigned int size'.
    * Check malloc doesn't return NULL before proceeding.

    EDIT:

    > How would you do this with functions that already have a return type, i.e. STACKpop?
    Return something that isn't going to occur -1 for example. Or, give it a success flag (example)... Or put the stackElement as an arg
    Code:
    stackElementT STACKpop(stackT * stack, int * result)
    {
        if(stack->count <= 0)
        {
            fprintf(stderr, "Cannot pop off empty stack.\n");
            *result = 1;
            /* garbage */
            return 0;                                                    
        }
        
        *result = 0;
        return stack->contents[--stack->count];                        
    }
    Then you'd only use the return value if result == 0.

    > How is this typically done?
    Code:
    ptr = malloc(blah);
    
    /* use ptr */
    
    free(ptr);
    Last edited by zacs7; 02-24-2008 at 02:49 AM.

  5. #5
    Registered User
    Join Date
    Jan 2008
    Posts
    69
    Thanks for reviewing!

    Quote Originally Posted by zacs7 View Post
    * use inclusion guards in stack.h
    What do you mean?

  6. #6
    Registered User
    Join Date
    Feb 2008
    Posts
    6
    Quote Originally Posted by cs32 View Post
    Thanks for reviewing!



    What do you mean?
    http://en.wikipedia.org/wiki/Include_guard

  7. #7
    Woof, woof! zacs7's Avatar
    Join Date
    Mar 2007
    Location
    Australia
    Posts
    3,459

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Fixing my program
    By Mcwaffle in forum C Programming
    Replies: 5
    Last Post: 11-05-2008, 02:55 AM
  2. C Code Review
    By trillianjedi in forum Projects and Job Recruitment
    Replies: 3
    Last Post: 06-18-2008, 12:29 PM
  3. stack and pointer problem
    By ramaadhitia in forum C Programming
    Replies: 2
    Last Post: 09-11-2006, 11:41 PM
  4. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 09:33 AM
  5. stack make file problem
    By puckett_m in forum C Programming
    Replies: 2
    Last Post: 11-22-2001, 10:51 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21