On 2008-09-29, arnuld <su*****@invalid.addresswrote:
>On Mon, 29 Sep 2008 11:18:54 +0100, Ben Bacarisse wrote:
>the array would be a char *** parameter).
yes, but then get_single_word() will take a char*** as argument. When I try to
do that I get invalid pointer error. I have to admit that this busines of
pointers is really hell confusing. get_words() is already getting the address of
the pointer then why have to pass the address of the address of the
pointer to get_single_word(). Even when I do that my program compiles
fine but ends in doing semantic error. Program exits as soon as you input
one word.
Since get_single_word() needs to return a pointer to a string (char *),
and does so "by reference", its parameter should be a char **. No more
indirection is necessary.
2nd, someone told me on clc that if a function crosses 40 lines then
alarms should be ringing. get_single_word() is a 60 line function. Does
it need to be abstracted into 2 other functions ?
Probably. Your style uses more vertical space than most, though, so
any line-count-related rules of thumb need to be adjusted for that.
Input functions are usually very short, 10 or 15 lines plus whatever
validation is necessary.
>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
enum { WORD_SIZE = 3, WORD_ERR = -1 };
enum { GSW_OK, GSW_ENOMEM, GSW_ENORESIZE } ;
enum { GI_OK = 0 , GI_ERR = WORD_ERR };
int get_words( char**, size_t* );
int get_single_word( char*** );
int main( void )
{
char* pword;
int input_err;
size_t words_count;
pword = NULL;
words_count = 0;
input_err = WORD_ERR; /* we will update the status in function
call to get_input() */
Do you not like initializing at declaration? It seems to me it would
be clearer to do so. (nb I wrapped your comment to fit in one line.)
input_err = get_words( &pword, &words_count );
if( WORD_ERR != input_err )
{
printf("words_count = %u\n", words_count);
}
size_t might be a different size than unsigned int, so you need to
cast it or use the %z specifier introduced in C99.
>
/* call to - sort_input(...)
call to - printf_input(...) */
free( pword );
return 0;
}
main() is fine other than those two minor nits.
>
int get_words( char** pword, size_t* w_cnt )
This seems suspicious - if get_words() needs to return a list of
strings, the required type is char ***. (Pointer to 'array' of
pointers to strings.)
{
int err_catcher;
*w_cnt = 0;
while( (err_catcher = get_single_word(&pword)) && ( **pword != 0) )
And then here you would pass *pword++, which gives get_single_word
a char ** to redirect to a string, and increments pword to point
to the next available item in the list.
{
if( GSW_OK == err_catcher )
{
++*w_cnt;
}
else
{
return GI_ERR;
}
}
return GI_OK;
}
int get_single_word( char*** ppc )
If you look, you find that you always refer to ppc with **ppc, which
suggests that a level of indirection can be removed.
{
unsigned ele_num;
int ch;
size_t word_length, word_length_interval;
char* word_begin;
char* new_mem;
ele_num = 0;
word_length = WORD_SIZE;
word_length_interval = 2;
**ppc = malloc(word_length * sizeof(**ppc));
word_begin = **ppc;
if( NULL == word_begin )
{
return GSW_ENOMEM;
}
while( (EOF != (ch = getchar())) && isspace(ch) )
{
continue; /* Leading whitespace */
}
if( EOF != ch )
{
*word_begin++ = ch;
ele_num = 1;
}
while( (EOF != (ch = getchar())) && (! isspace(ch)) )
{
if( (word_length - 1) == ele_num++ )
{
new_mem = realloc( **ppc, (word_length_interval * word_length * sizeof *new_mem) );
if( new_mem )
{
word_begin = new_mem + (word_begin - **ppc);
word_length *= word_length_interval;
**ppc = new_mem;
}
else
{
*word_begin = '\0';
return GSW_ENORESIZE;
}
}
*word_begin++ = ch;
}
This whole loop needs to be redone using my suggestions, as well as those of
other posters.
>
*word_begin = '\0';
return GSW_OK;
}
--
Andrew Poelstra
ap*******@wpsoftware.net
Only GOD may divide by zero. That is how he created black holes.
-Veselin Jungic
