LIFO in C, need your suggestions

S

Sumit

created a LIFO in C, need your suggestion what else more can i do on it,
and what should i change to make it best. need your suggestion specially
in function - stack_is_empty() , stack_remove() and stack_free() i have
doubt on those function.

I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function.
is it good idea?
Thanks



#include <stdio.h>
#include <stdlib.h>

enum
{
SIZE_ARR = 20,
FAIL = -1,
NO = 0,
YES = 1
};

struct sstack_node
{
char name[SIZE_ARR];
struct sstack_node * next;
};

struct sstack_list
{
struct sstack_node * top;
};


/*--- FUNCTION DICLEARTION -----*/

struct sstack_list * stack_init( void );
void push(struct sstack_list *s , char * );
struct sstack_node * pop(struct sstack_list * s);

int stack_is_empty(struct sstack_list* s);
struct sstack_list * stack_refresh(struct sstack_list* s);
struct sstack_node * stack_frame(char *);
void stack_remove(struct sstack_list *);
void stack_free(struct sstack_list *);
int copy(char * to, char * from);
/*-------------------------------------*/


int main( void )
{
struct sstack_list * stack = NULL;
struct sstack_node * stnode = NULL;

stack = stack_init(); /* initializing new stack */

/* pushing some string into stack */
push(stack, "comp");
push(stack, "dot");
push(stack, "lang");
push(stack, "dot");
push(stack, "c");
push(stack, "programming");
push(stack, "unix");
push(stack, "linux");

/* poping */
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n\n", stnode -> name);

if( YES == stack_is_empty(stack) )
stack_refresh(stack);
else
printf("stack is not empty yet \n");
stack_remove(stack);

return 0;
}

void stack_remove(struct sstack_list * s )
{
if( s == NULL)
{
printf(" there is no stack list \n");
return;
}

if( s -> top != NULL )
{
printf(" stack is not empty yet, still making it empty \n");
pop(s);
}
stack_free( s );
printf("stack has been removed successfully \n");
}

void stack_free(struct sstack_list * sfree)
{
if( sfree != NULL)
{
free( sfree );
sfree = NULL;
return;
}
printf("address is already free \n");
return ;

}

struct sstack_node* pop(struct sstack_list * s)
{
struct sstack_node * temp = NULL;

if( s == NULL)
{
printf("stack is not initialized \n");
return NULL;
}

if( s -> top == NULL )
{
printf("Stack is empty \n");
return NULL;
}
temp = s -> top;
if(s -> top == NULL)
{
printf("s t a c k - e m p t y \n");
return NULL;
}
s -> top = s -> top -> next ;
return temp;
}

void push(struct sstack_list *s , char *name )
{
struct sstack_node * newframe = NULL;
newframe = stack_frame(name); /* got a new node for insert into stack */
if( NULL == newframe ) return;
if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}
}

struct sstack_node * stack_frame( char * name)
{
struct sstack_node * newframe = NULL;
newframe = ( struct sstack_node * ) malloc (1 * sizeof *newframe);
if( NULL == newframe )
{
fprintf(stderr,"error: malloc fail \n");
return NULL;
}
copy( newframe -> name , name);
newframe -> next = NULL;
return newframe;
}

int copy(char to[], char from[])
{
int i=0;
while(( to = from ) != '\0')
++i;

to='\0';
return i;
}

struct sstack_list* stack_refresh(struct sstack_list* s)
{
if( s == NULL )
{
printf("stack is not initlialized \n ");
return NULL;
}
if( s -> top != NULL)
{
printf("stack is not empty cannot refresh stack until it is not be empty \n");
return s;
}
s -> top = NULL;
printf("stack has been refreshed successfully \n");
return s;
}

int stack_is_empty(struct sstack_list* s)
{
if( NULL == s )
{
printf("stack was not initlialized \n");
return FAIL;
}
else if( NULL == s -> top )
{
printf("stack is empty \n");
return YES;
}
else
{
printf("stack is not empty \n");
return NO;
}
printf("%s:%d impossible condition \n", __FILE__, __LINE__);
return NO;
}

struct sstack_list * stack_init( void )
{
struct sstack_list * s = NULL;
s = (struct sstack_list *) malloc (1 * sizeof *s);
if( NULL == s )
{
fprintf(stderr,"error: malloc fail \n");
return s;
}
s -> top = NULL;
return s;
}
 
T

Tinku

created a LIFO in C, need your suggestion what else more can i do on it,
and what should i change to make it best. need your suggestion specially
in function - stack_is_empty() , stack_remove() and stack_free() i have
doubt on those function.

I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function.
is it good idea?
Thanks

#include <stdio.h>
#include <stdlib.h>

enum
{
SIZE_ARR = 20,
FAIL = -1,
NO = 0,
YES = 1

};

struct sstack_node
{
char name[SIZE_ARR];
struct sstack_node * next;

};

struct sstack_list
{
struct sstack_node * top;

};

/*--- FUNCTION DICLEARTION -----*/

struct sstack_list * stack_init( void );
void push(struct sstack_list *s , char * );
struct sstack_node * pop(struct sstack_list * s);

int stack_is_empty(struct sstack_list* s);
struct sstack_list * stack_refresh(struct sstack_list* s);
struct sstack_node * stack_frame(char *);
void stack_remove(struct sstack_list *);
void stack_free(struct sstack_list *);
int copy(char * to, char * from);
/*-------------------------------------*/

int main( void )
{
struct sstack_list * stack = NULL;
struct sstack_node * stnode = NULL;

stack = stack_init(); /* initializing new stack */

/* pushing some string into stack */
push(stack, "comp");
push(stack, "dot");
push(stack, "lang");
push(stack, "dot");
push(stack, "c");
push(stack, "programming");
push(stack, "unix");
push(stack, "linux");

/* poping */
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n\n", stnode -> name);

if( YES == stack_is_empty(stack) )
stack_refresh(stack);
else
printf("stack is not empty yet \n");
stack_remove(stack);

return 0;

}

void stack_remove(struct sstack_list * s )
{
if( s == NULL)
{
printf(" there is no stack list \n");
return;
}

if( s -> top != NULL )
{
printf(" stack is not empty yet, still making it empty \n");
pop(s);
}
stack_free( s );
printf("stack has been removed successfully \n");

}

void stack_free(struct sstack_list * sfree)
{
if( sfree != NULL)
{
free( sfree );
sfree = NULL;
return;
}
printf("address is already free \n");
return ;

}

struct sstack_node* pop(struct sstack_list * s)
{
struct sstack_node * temp = NULL;

if( s == NULL)
{
printf("stack is not initialized \n");
return NULL;
}

if( s -> top == NULL )
{
printf("Stack is empty \n");
return NULL;
}
temp = s -> top;
if(s -> top == NULL)
{
printf("s t a c k - e m p t y \n");
return NULL;
}
s -> top = s -> top -> next ;
return temp;

}

void push(struct sstack_list *s , char *name )
{
struct sstack_node * newframe = NULL;
newframe = stack_frame(name); /* got a new node for insert into stack */
if( NULL == newframe ) return;
if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}

}

struct sstack_node * stack_frame( char * name)
{
struct sstack_node * newframe = NULL;
newframe = ( struct sstack_node * ) malloc (1 * sizeof *newframe);
if( NULL == newframe )
{
fprintf(stderr,"error: malloc fail \n");
return NULL;
}
copy( newframe -> name , name);
newframe -> next = NULL;
return newframe;

}

int copy(char to[], char from[])
{
int i=0;
while(( to = from ) != '\0')
++i;

to='\0';
return i;

}

struct sstack_list* stack_refresh(struct sstack_list* s)
{
if( s == NULL )
{
printf("stack is not initlialized \n ");
return NULL;
}
if( s -> top != NULL)
{
printf("stack is not empty cannot refresh stack until it is not be empty \n");
return s;
}
s -> top = NULL;
printf("stack has been refreshed successfully \n");
return s;

}

int stack_is_empty(struct sstack_list* s)
{
if( NULL == s )
{
printf("stack was not initlialized \n");
return FAIL;
}
else if( NULL == s -> top )
{
printf("stack is empty \n");
return YES;
}
else
{
printf("stack is not empty \n");
return NO;
}
printf("%s:%d impossible condition \n", __FILE__, __LINE__);
return NO;

}

struct sstack_list * stack_init( void )
{
struct sstack_list * s = NULL;
s = (struct sstack_list *) malloc (1 * sizeof *s);
if( NULL == s )
{
fprintf(stderr,"error: malloc fail \n");
return s;
}
s -> top = NULL;
return s;

}
Click to expand...


Sorry I forgot to give output in last post
here it is -
============== OUTPUT =====================
POPED: linux
POPED: unix
POPED: programming
POPED: c
POPED: dot
POPED: lang
POPED: dot
POPED: comp

stack is empty
stack has been refreshed successfully
stack has been removed successfully
====================================
 
A

arnuld

I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function. is it
good idea?
Click to expand...

I am sure other will comment on this.


/*--- FUNCTION DICLEARTION -----*/
Click to expand...
int stack_is_empty(struct sstack_list* s);
Click to expand...

This function is a boolean, so I guess right name must be is_stack_empty()


void stack_remove(struct sstack_list * s ) {
if( s == NULL)
{
printf(" there is no stack list \n"); return;
}

if( s -> top != NULL )
{
printf(" stack is not empty yet, still making it empty \n");
pop(s);
}
stack_free( s );
printf("stack has been removed successfully \n");
}
Click to expand...


I wonder what exactly is the difference between stack_remove() and
stack_free(). You don't explain that. A very general and random
interpretation will be stack_remove() will remove the stack and stack_free
() will free the stack, which means stack_remove() only removes the stack
but does not free the memory held by its different elements ? And when
all the elements are removed how you will free() them when pointers are
gone ? I am sure someone will make some different meaning out of it and
it will not be his mistake. I am not even saying your style is wrong, Its
just that you did not tell *us* what they do. Put a line or 2 for
comments. PURPOSE: ...... WHAT IT DOES: ..... WHAT I WANT: ....
WHAT HAPPEND: ....



void stack_free(struct sstack_list * sfree) {
if( sfree != NULL)
{
free( sfree );
sfree = NULL;
return;
}
printf("address is already free \n"); return ;

}
Click to expand...


struct sstack_node* pop(struct sstack_list * s) {
struct sstack_node * temp = NULL;

if( s == NULL)
{
printf("stack is not initialized \n"); return NULL;
}

if( s -> top == NULL )
{
printf("Stack is empty \n");
return NULL;
}
temp = s -> top;
if(s -> top == NULL)
{
printf("s t a c k - e m p t y \n");
return NULL;
}
s -> top = s -> top -> next ;
return temp;
}
Click to expand...

I see people using pop() 2 ways. One that always free()s the element
after popping it (me) and the other when it pops but does not free()
(you).
Just telling you, keep this on mind.


void push(struct sstack_list *s , char *name ) {
struct sstack_node * newframe = NULL; newframe = stack_frame(name);
/* got a new node for insert into stack */ if( NULL == newframe )
return;
if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}
}
Click to expand...

void push(struct sstack_list *s , char *name )
{
struct sstack_node * newframe = malloc( sizeof * newframe * 1) ;
if( NULL == newframe )
{
fprintf(stderr, "FILE: %s, LINE: %d : malloc() failed\n",
__FILE__, __LINE);
return;
}

newframe->next = NULL
strcpy(newframe, name);

if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}
}

Its just 3 lines longer than yours. It removes the need of stack_frame().
I only make a new function when either there is a special need or when
the lines of code increase beyond 40. Its a matter of choice I guess but
I will like to know the viewpoint of others.


int copy(char to[], char from[])
{
int i=0;
while(( to = from ) != '\0')
++i;

to='\0';
return i;
}
Click to expand...



I really wonder why you need that ? strcpy() does that same. Why are you
reinventing the wheel ? You have included *whole* of stdlib.h but then
used only one thing from it (malloc), then why you scared of including
string.h ?


struct sstack_list* stack_refresh(struct sstack_list* s) {
if( s == NULL )
{
printf("stack is not initlialized \n "); return NULL;
}
if( s -> top != NULL)
{
printf("stack is not empty cannot refresh stack until it is not
be empty \n"); return s;
}
s -> top = NULL;
printf("stack has been refreshed successfully \n"); return s;
}
Click to expand...

Whats the meaning of stack_refresh ? .. you have stack with 100 elements
and you call stack_refresh() which sets the top to null without fee()ing
the memory held by 100 elements.


struct sstack_list * stack_init( void ) {
struct sstack_list * s = NULL;
s = (struct sstack_list *) malloc (1 * sizeof *s); if( NULL == s )
Click to expand...


C-FAQs 7.7b
 
B

Ben Bacarisse

Sumit said:
created a LIFO in C, need your suggestion what else more can i do on it,
and what should i change to make it best. need your suggestion specially
in function - stack_is_empty() , stack_remove() and stack_free() i have
doubt on those function.

I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function.
is it good idea?
Click to expand...

I don't think so. I can't see what advantage you think you gain by
doing the extra work (and thereby maybe introduce more bugs).
#include <stdio.h>
#include <stdlib.h>

enum
{
SIZE_ARR = 20,
FAIL = -1,
NO = 0,
YES = 1
};
Click to expand...

I'd leave 0 and 1 unnamed and use the results in Boolean tests without
== but I see that you want to return FAIL in some cases. I think this
complicates the use with little benefit. I want to write tests like
this:

while (!stack_is_empty(s)) { /* ... */ }

The empty test can return FAIL but that is not interesting to anyone.
It can only fail when s == NULL which I can test myself. If there are
more complex failures, you will need a more general mechanism anyway
so this simple YES/NO/FAIL return is of limited use.
struct sstack_node
{
char name[SIZE_ARR];
struct sstack_node * next;
};

struct sstack_list
{
struct sstack_node * top;
};
Click to expand...
void stack_remove(struct sstack_list * s )
{
if( s == NULL)
{
printf(" there is no stack list \n");
return;
Click to expand...

Why, having decided to have a FAIL return why does this function not
return FAIL?

I assume all the printing going on is just for testing, yes? General-use
functions should not normally print anything.
}

if( s -> top != NULL )
{
printf(" stack is not empty yet, still making it empty \n");
pop(s);
}
Click to expand...

Eh? Did you mean 'while' rather than 'if' here? You should use still
your test, stack_is_empty, even in your own code!
stack_free( s );
printf("stack has been removed successfully \n");
}

void stack_free(struct sstack_list * sfree)
{
if( sfree != NULL)
{
free( sfree );
sfree = NULL;
Click to expand...

I would not assign to sfree here, but I accept that some people feel
they must.
return;
}
printf("address is already free \n");
return ;

}

struct sstack_node* pop(struct sstack_list * s)
{
struct sstack_node * temp = NULL;

if( s == NULL)
{
printf("stack is not initialized \n");
return NULL;
}

if( s -> top == NULL )
{
printf("Stack is empty \n");
return NULL;
}
temp = s -> top;
if(s -> top == NULL)
{
printf("s t a c k - e m p t y \n");
return NULL;
}
Click to expand...

Duplicate test here.
s -> top = s -> top -> next ;
return temp;
}

void push(struct sstack_list *s , char *name )
{
struct sstack_node * newframe = NULL;
newframe = stack_frame(name); /* got a new node for insert into stack */
if( NULL == newframe ) return;
if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}
Click to expand...

You don't need to test for an empty stack.

newframe->next = s->top;
s->top = newframe;

works for both cases.
}

struct sstack_node * stack_frame( char * name)
{
struct sstack_node * newframe = NULL;
newframe = ( struct sstack_node * ) malloc (1 * sizeof *newframe);
if( NULL == newframe )
{
fprintf(stderr,"error: malloc fail \n");
return NULL;
}
copy( newframe -> name , name);
Click to expand...

You've written your own copy but not taken the chance to make it
safer! copy could be told how large the target array is.
newframe -> next = NULL;
return newframe;
}

int copy(char to[], char from[])
{
int i=0;
while(( to = from ) != '\0')
++i;

to='\0';
Click to expand...


This line is pointless. The loop can only stop when to is set to
zero.
return i;
}

struct sstack_list* stack_refresh(struct sstack_list* s)
{
if( s == NULL )
{
printf("stack is not initlialized \n ");
return NULL;
}
if( s -> top != NULL)
{
printf("stack is not empty cannot refresh stack until it is not be empty \n");
return s;
}
s -> top = NULL;
printf("stack has been refreshed successfully \n");
return s;
}
Click to expand...

What is this function for? It seems to set s->top to NULL but only if
s->top already NULL.
int stack_is_empty(struct sstack_list* s)
{
if( NULL == s )
{
printf("stack was not initlialized \n");
return FAIL;
}
else if( NULL == s -> top )
{
printf("stack is empty \n");
return YES;
}
else
{
printf("stack is not empty \n");
return NO;
}
printf("%s:%d impossible condition \n", __FILE__, __LINE__);
return NO;
}

struct sstack_list * stack_init( void )
{
struct sstack_list * s = NULL;
s = (struct sstack_list *) malloc (1 * sizeof *s);
if( NULL == s )
{
fprintf(stderr,"error: malloc fail \n");
return s;
}
s -> top = NULL;
return s;
}
Click to expand...

I find your code slightly 'wordy'. I'd write:

struct sstack_list *stack_init(void)
{
struct sstack_list *s = malloc(sizeof *s);
if (s)
s->top = NULL;
else fprintf(stderr, "error: malloc fail\n");
return s;
}

but that is a matter of taste.
 
K

Keith Thompson

arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int stack_is_empty(struct sstack_list* s);
Click to expand...

This function is a boolean, so I guess right name must be is_stack_empty()
Click to expand...
[...]

Why is "is_stack_empty" a better name than "stack_is_empty"?
 
L

Lew Pitcher

arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int stack_is_empty(struct sstack_list* s);
Click to expand...

This function is a boolean, so I guess right name must be
is_stack_empty()
Click to expand...
[...]

Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...

Presumably because, in English
"stack is empty"
is an assertion, needing no response, while
"is stack empty"
is a question, with a boolean (yes/no) response.

Correspondingly,
is_stack_empty()
could be understood by the programmer to answer the question of "is the
stack empty" with a boolean result.

--
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
---------- Slackware - Because I know what I'm doing. ------
 
B

Beej Jorgensen

Sumit said:
I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function.
is it good idea?
Click to expand...

Generally not a good idea, for reasons others have said. Using built-in
well-tested and well-optimized functions like strcpy() is usually the
way to go.

#including a file doesn't tend to bloat things. But do a build with
your stuff and another build with <string.h>/strcpy() and see how they
compare sizewise.

Also, do a speed test with your copy() vs. strcpy().

My guess is that the binary won't be appreciably larger with strcpy(),
and it might even be smaller. And I'd bet strcpy() will, in general,
run about 4-8x faster than your copy() depending on your architecture.

-Beej
 
K

Keith Thompson

Lew Pitcher said:
arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int stack_is_empty(struct sstack_list* s);

This function is a boolean, so I guess right name must be
is_stack_empty()
Click to expand...
[...]

Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...

Presumably because, in English
"stack is empty"
is an assertion, needing no response, while
"is stack empty"
is a question, with a boolean (yes/no) response.

Correspondingly,
is_stack_empty()
could be understood by the programmer to answer the question of "is the
stack empty" with a boolean result.
Click to expand...

Consider the context:
if (stack_is_empty(s)) {
/* ... */
}
vs.
if (is_stack_empty(s)) {
/* ... */
}

I actually have no strong preference for one over the other, but
if we're trying to match English grammar I'd say "stack_is_empty"
works better.
 
B

Ben Bacarisse

Keith Thompson said:
Consider the context:
if (stack_is_empty(s)) {
/* ... */
}
vs.
if (is_stack_empty(s)) {
/* ... */
}

I actually have no strong preference for one over the other, but
if we're trying to match English grammar I'd say "stack_is_empty"
works better.
Click to expand...

What's more, in the absence of name-spaces, is it often a good idea to
start the name of all related functions with some common string:
stack_pop, stack_push and so on. I image the OP chose stack_is_empty
for exactly this reason though of course I am just guessing.
 
T

Tinku

Lew Pitcher said:
On August 5, 2009 12:14, in comp.lang.c, Keith Thompson ([email protected])
wrote:
]
int   stack_is_empty(struct sstack_list* s);
This function is a boolean, so I guess right name must be
is_stack_empty()
[...]
Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...
Click to expand...
Presumably because, in English
  "stack is empty"
is an assertion, needing no response, while
  "is stack empty"
is a question, with a boolean (yes/no) response.
Click to expand...
Correspondingly,
  is_stack_empty()
could be understood by the programmer to answer the question of "is the
stack empty" with a boolean result.
Click to expand...

Consider the context:
    if (stack_is_empty(s)) {
        /* ... */
    }
vs.
    if (is_stack_empty(s)) {
        /* ... */
    }

I actually have no strong preference for one over the other, but
if we're trying to match English grammar I'd say "stack_is_empty"
works better.

--
Keith Thompson (The_Other_Keith) (e-mail address removed)  <http://www.ghoti.net/~kst>
Nokia
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
Click to expand...



Thanks Arnuld , Ben and Beej

now function name stack_is_empty() has came to picture ....
actually I took this name because of maintaining similarity of
name with other functions accept push and pop but question is,
why prefix "stack_" has not been used in push() and pop() function ?
because it should be easy to invoke.

we should not give any chance to our confusion, about function name -
stack_is_empty() and is_stack_empty()
because according to English Grammar is_stack_empty? is a kind of
Interrogative
Sentence. but here invoking function is not asking any question to
stack controller,
rather then here a command is being sent where the answer will be
given in
YES/NO. but anyway i will keep this thing in my mind, Thanks .
 
P

Phil Carmody

Eric Sosman said:
Keith said:
arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int stack_is_empty(struct sstack_list* s);
This function is a boolean, so I guess right name must be is_stack_empty()
Click to expand...
[...]

Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...

It's not. Both should be stackemptyp, returning t or nil.

(Anybody got a spare parenthesis key? Mine's almost worn out.)
Click to expand...

Brace yourself...












....









....




Nope.

However, I do have the final page of the leaked source code
for Ronnie Raygun's Strategic Defence Initiative (SDI, a.k.a. Star
Wars) project - it was indeed written in lisp!

-- 8< --
)))
)))))))))
))))))))))))))
)))))))
)))))
)))))))))
))))))
))))))
)))))))
)
-- 8< --
 
P

Phil Carmody

Malcolm McLean said:
Code that doesn't depend on anything is a lot better than code which depends
on the standard library, or on some other source file. You can use it in any
environment.
Click to expand...

I just compiled a program which did something using a hand-rolled
strcpy()-alike on my POWER box, and it *completely* refused to run
on my ARM or my Alpha.

Please help me use it in these other environments.

Phil
 
D

Default User

Eric said:
Keith said:
arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int stack_is_empty(struct sstack_list* s);
This function is a boolean, so I guess right name must be
is_stack_empty()
Click to expand...
[...]

Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...

It's not. Both should be stackemptyp, returning t or nil.
Click to expand...

Queries ending in 'p' make me think of John Varley's SF story, "Press
Enter¦".



Brian
 
B

Barry Schwarz

created a LIFO in C, need your suggestion what else more can i do on it,
and what should i change to make it best. need your suggestion specially
in function - stack_is_empty() , stack_remove() and stack_free() i have
doubt on those function.

I wrote one more extra function copy(), because for using only one
function "strcpy()" from string library unnecessary i have to include
whole library <string.h> , that is what i wrote copy() function.
is it good idea?
Click to expand...

You don't include libraries. You include headers which are processed
during the compilation phase.

Libraries are processed during the link phase. While not guaranteed,
I would bet that almost ever linker in use will only add the functions
you actually use to your program regardless of how many unused ones
are in the library.
Thanks



#include <stdio.h>
#include <stdlib.h>

enum
{
SIZE_ARR = 20,
FAIL = -1,
NO = 0,
YES = 1
};

struct sstack_node
{
char name[SIZE_ARR];
struct sstack_node * next;
};

struct sstack_list
{
struct sstack_node * top;
};
Click to expand...

What benefit do you derive from a structure with a single member?
Every place you use a struct sstack_list you could just as easily use
a struct sstack_node * and eliminate a lot of -> constructs.
/*--- FUNCTION DICLEARTION -----*/

struct sstack_list * stack_init( void );
void push(struct sstack_list *s , char * );
struct sstack_node * pop(struct sstack_list * s);

int stack_is_empty(struct sstack_list* s);
struct sstack_list * stack_refresh(struct sstack_list* s);
struct sstack_node * stack_frame(char *);
void stack_remove(struct sstack_list *);
void stack_free(struct sstack_list *);
int copy(char * to, char * from);
/*-------------------------------------*/


int main( void )
{
struct sstack_list * stack = NULL;
struct sstack_node * stnode = NULL;

stack = stack_init(); /* initializing new stack */
Click to expand...

You never check to see if stack_init succeeded.
/* pushing some string into stack */
push(stack, "comp");
Click to expand...

You have no way of knowing if push succeeded.
push(stack, "dot");
push(stack, "lang");
push(stack, "dot");
push(stack, "c");
push(stack, "programming");
push(stack, "unix");
push(stack, "linux");

/* poping */
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n", stnode -> name);
stnode = pop(stack);
printf("POPED: %s \n\n", stnode -> name);

if( YES == stack_is_empty(stack) )
stack_refresh(stack);
else
printf("stack is not empty yet \n");
stack_remove(stack);

return 0;
}

void stack_remove(struct sstack_list * s )
{
if( s == NULL)
{
printf(" there is no stack list \n");
return;
}

if( s -> top != NULL )
Click to expand...

You probably meant while instead of if as others have mentioned.
{
printf(" stack is not empty yet, still making it empty \n");
pop(s);
}
stack_free( s );
printf("stack has been removed successfully \n");
}

void stack_free(struct sstack_list * sfree)
{
if( sfree != NULL)
{
free( sfree );
sfree = NULL;
Click to expand...

sfree is local to this function. Setting it to an unused value serves
no purpose.
return;
}
printf("address is already free \n");
return ;

}

struct sstack_node* pop(struct sstack_list * s)
{
struct sstack_node * temp = NULL;

if( s == NULL)
{
printf("stack is not initialized \n");
return NULL;
}

if( s -> top == NULL )
{
printf("Stack is empty \n");
return NULL;
}
temp = s -> top;
if(s -> top == NULL)
{
printf("s t a c k - e m p t y \n");
return NULL;
}
s -> top = s -> top -> next ;
return temp;
}

void push(struct sstack_list *s , char *name )
{
struct sstack_node * newframe = NULL;
newframe = stack_frame(name); /* got a new node for insert into stack */
if( NULL == newframe ) return;
if( NULL == s -> top ) /* chekcing first frame into stack */
s -> top = newframe;
else
{
newframe -> next = s -> top;
s -> top = newframe;
}
}

struct sstack_node * stack_frame( char * name)
{
struct sstack_node * newframe = NULL;
newframe = ( struct sstack_node * ) malloc (1 * sizeof *newframe);
Click to expand...

What purpose do you think the cast serves?
if( NULL == newframe )
{
fprintf(stderr,"error: malloc fail \n");
return NULL;
}
copy( newframe -> name , name);
newframe -> next = NULL;
return newframe;
}

int copy(char to[], char from[])
{
int i=0;
while(( to = from ) != '\0')
++i;

to='\0';
Click to expand...


Redundant as has been mentioned.
return i;
}

struct sstack_list* stack_refresh(struct sstack_list* s)
{
if( s == NULL )
{
printf("stack is not initlialized \n ");
return NULL;
}
if( s -> top != NULL)
{
printf("stack is not empty cannot refresh stack until it is not be empty \n");
return s;
}
s -> top = NULL;
Click to expand...

If there were nodes in the stack, this causes a memory leak.
printf("stack has been refreshed successfully \n");
return s;
}

int stack_is_empty(struct sstack_list* s)
{
if( NULL == s )
{
printf("stack was not initlialized \n");
return FAIL;
}
else if( NULL == s -> top )
{
printf("stack is empty \n");
return YES;
}
else
{
printf("stack is not empty \n");
return NO;
}
printf("%s:%d impossible condition \n", __FILE__, __LINE__);
Click to expand...

This code cannot be reached.
 
A

arnuld

... SNIP..
why prefix "stack_" has not been used in push() and pop() function ?
because it should be easy to invoke.
Click to expand...

I don't agree with that and that may be a matter of style. If I am
implementing a Data-Structure in C then I usually name all the
functions with a prefix of that data structure.

we should not give any chance to our confusion, about function name -
stack_is_empty() and is_stack_empty()
because according to English Grammar is_stack_empty? is a kind of
Interrogative
Sentence. but here invoking function is not asking any question to
stack controller,
rather then here a command is being sent where the answer will be
given in
YES/NO. but anyway i will keep this thing in my mind, Thanks .
Click to expand...

and when do you get a yes/no answer, when someone asks you a question.
So your function is asking a question, stack is empty or not ? ...
which only be answered in yes/no which makes it Boolean. But I agree
with Ben here, all function names need to bear similarity like using
the same prefix, stack in your case. I mean you need to decide on one
way or the other either use prefix or not. You use it in half of the
functions but not in others and that is why I recommended
is_stack_empty().
 
A

arnuld

Presumably because, in English
"stack is empty"
is an assertion, needing no response, while
"is stack empty"
is a question, with a boolean (yes/no) response.

Correspondingly,
is_stack_empty()
could be understood by the programmer to answer the question of "is the
stack empty" with a boolean result.
Click to expand...

Exactly my point. Actually OP is not using common prefix like I usually
use. stack_is_empty() looks good when you are using a prefix, stack_pop,
stack_push, stack_free. He is using prefix in half of the places, so I am
sure he does not stick to some convention of naming functions. If prefix
would have been used in all of the functions, then stack_is_empty() would
be my choice.
 
N

Nick Keighley

Keith said:
arnuld said:
On Wed, 05 Aug 2009 11:38:17 +0000, Sumit wrote: [...]
int   stack_is_empty(struct sstack_list* s);
This function is a boolean, so I guess right name must be is_stack_empty() [...]
Click to expand...

Why is "is_stack_empty" a better name than "stack_is_empty"?
Click to expand...

     It's not.  Both should be stackemptyp, returning t or nil.

     (Anybody got a spare parenthesis key?  Mine's almost worn out.)
Click to expand...

ITYM

stack-empty? returning #t or #f
 
D

Default User

pete said:
Tinku wrote:

Call it stack_empty.

It works with your convention and

if (stack_empty(s)) /* isn't really awkward */
Click to expand...


All the coding standards I've used here at work mandate verb phrases
for function names. It was tough when I first started working under
such rules, as I was used to C standard function names, which were for
the most part not verb phrases.



Brian
 
G

Giorgos Keramidas

created a LIFO in C, need your suggestion what else more can i do on
it, and what should i change to make it best. need your suggestion
specially in function - stack_is_empty() , stack_remove() and
stack_free() i have doubt on those function.
Click to expand...

There are nice macros for singly-linked lists, doubly-linked lists and
tail-queues in BSD and Linux. A tail-queue can serve as a very nice
LIFO if you wrap it in code of your own to insert elements at the head
and remove them from the tail (or vice versa).

Have a look at /usr/include/sys/queue.h in FreeBSD and Linux.

It's often a good idea to *avoid* reimplementing a particular wheel,
especially if there is a possibility for amusing ``corners'' (bugs,
limitations, or lack of features) in the new wheel reimplementation :)
 
G

Guest

|> created a LIFO in C, need your suggestion what else more can i do on
|> it, and what should i change to make it best. need your suggestion
|> specially in function - stack_is_empty() , stack_remove() and
|> stack_free() i have doubt on those function.
|
| There are nice macros for singly-linked lists, doubly-linked lists and
| tail-queues in BSD and Linux. A tail-queue can serve as a very nice
| LIFO if you wrap it in code of your own to insert elements at the head
| and remove them from the tail (or vice versa).
|
| Have a look at /usr/include/sys/queue.h in FreeBSD and Linux.
|
| It's often a good idea to *avoid* reimplementing a particular wheel,
| especially if there is a possibility for amusing ``corners'' (bugs,
| limitations, or lack of features) in the new wheel reimplementation :)

OTOH, it can be good to reimplement when the new one fixes bugs in the
legacy implementation. The queue.h implementation does not protect against
multiple evaluation of arguments. I have code that does. The tradeoff is
that my code is not as widely tested (that's what you get with any new code).
In fact, these are not even officially released. So of course I'd like to
see it tested more widely. You can preview (and even use) them here.

The linked list macro collection by itself:
http://phil.ipal.org/pre-release/list-0.0.6/list.h

The linked list macro collection in a larger collection:
http://phil.ipal.org/pre-release/dropinc-0.0.0/misc/list.i

These are now licensed under the BSD-like license used by OpenBSD.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Implementing Many Stacks in the Same Program 1
Array implementation of Stack 80
LIFO in C 11
Stack using Array 16
HELP:function at c returning (null) 3
Linux: using "clone3" and "waitid" 0
Need help in debugging tic tac toe (Beginner) 0
Command Line Arguments 0

Members online

Total: 37 (members: 3, guests: 34)
Robots: 85

Forum statistics

Threads
473,754
Messages
2,569,522
Members
44,995
Latest member
PinupduzSap

Latest Threads

Top