patterncMinor
Returning the length of a 'hidden' string within each of the sentences of another string
Viewed 0 times
hiddentheeachlengthwithinreturninganothersentencesstring
Problem
The following program is supposed to return the length of a 'hidden' string within each of the sentences of another string, using pointers and avoiding as much as possible the use of
[] as a means to access strings. We were instructed on how to declare the functions, hence their declarations are a given.#include
#include
int getHiddenString(const char * str1 , const char * str2);
int divideStringIntoSentences(char * string, char * sentences[]);
char* readLine();
int main()
{
char *ptrarr[20];
int i = 0;
printf ("Please enter a string:\n");
char *str1 = readLine();
printf ("Please enter a hidden string:\n");
char *str2 = readLine();
if (str2 == NULL) return EXIT_FAILURE;
int j = divideStringIntoSentences(str1, ptrarr);
for (; i <= j; i++)
printf ("Sentence %d contains hidden string prefix length %d\n", i,
getHiddenString(*(ptrarr + i), str2));
free (str1);
free (str2);
return 0;
}
int getHiddenString(const char * str1 , const char * str2)
{
int hidlen = 0;
for (; *str1 != '\0' && *str2 != '\0'; str1++)
if (*str1 == *str2)
{
hidlen++;
str2++;
}
return (hidlen);
}
char* readLine()
{
int capacity = 1, index = 0;
char c;
char *str1 = (char*) malloc(capacity * sizeof(char));
if (str1 == NULL) return NULL;
for (c = getchar(); c != '\n' && capacity <= 100; c = getchar())
{
if (index == capacity - 1)
{
str1 = (char*) realloc(str1, (++capacity) * 2 * sizeof(char));
if (str1 == NULL) return NULL;
capacity*=2;
}
*(str1 + index++) = c;
}
*(str1 + index) = '\0';
return str1;
}
int divideStringIntoSentences(char * string, char * sentences[])
{
int sentnum = 0;
*sentences = string;
for (; *string != '\0'; string++)
if (*string == '.')
{
*string = '\0';
*(++sentences) = (string + 1);
sentnum++;
}
return (sentnum);
}Solution
Some generals comments:
Define functions in reverse order to avoid prototypes, make local functions static, add
In function
This function has many issues, some minor, such as casting
new line of 100 chars. In your variable name,
as you only use one string. And your default sentence allocation of 1 char is
silly - start with something reasonable, say 64 (arbitrary value).
More seriously,
Your reallocation should compute the new capacity then use that in the
call rather than computing it twice. And your
stands leaks memory on failure - the original string
In function
You need to pass the size of
detection.
In function
Parameter names of
do. If it is supposed to find the hidden string in the sentence, then it
doesn't work. Library function
In
Although the functions handle malloc failures,
but not including
"Sentence # contains hidden string prefix length #" is printed even if the
sentence doesn't contain the hidden string. This make me think that the
the hidden string contained in the sentence, or some such. In that case it
still doesn't work and is badly named (eg if I give it "two twa twe" and the
hidden string "twe" it says, "Sentence 0 contains hidden string prefix length 4"
Define functions in reverse order to avoid prototypes, make local functions static, add
void to empty parameter lists, use braces even when unnecessary, use spaces consistently around operators, define a single variable per line, return statements don't need brackets (you are inconsistent in this).In function
readLineThis function has many issues, some minor, such as casting
malloc, usingsizeof(char) (sizeof char is 1 by definition) and the arbitrary limit on anew line of 100 chars. In your variable name,
str1, the '1' is unnecessary,as you only use one string. And your default sentence allocation of 1 char is
silly - start with something reasonable, say 64 (arbitrary value).
More seriously,
getchar returns an int not char. This allows it to hold the value for EOF, the end-of-file marker. Your for-loop should test for EOF. If you fail to do this then if you input from a redirected file, the program will fail if EOF is reached without finding a \n. A better loop would be, for example:int ch;
while((ch = getchar()) != EOF) {
if (ch == '\n') {
break;
}
// do the necessary stuff
}Your reallocation should compute the new capacity then use that in the
realloccall rather than computing it twice. And your
realloc call as it currentlystands leaks memory on failure - the original string
str1 is not freed byrealloc on failure.In function
divideStringIntoSentencesYou need to pass the size of
sentences[] into the function to allow for overflowdetection.
In function
getHiddenStringParameter names of
sentence and hidden might be more understandable thanstr1, str2. More seriously, I don't know what it is supposed todo. If it is supposed to find the hidden string in the sentence, then it
doesn't work. Library function
strstr would do the job for you.In
mainAlthough the functions handle malloc failures,
main does not check forstr1 in the returned values. Again str1 and str2 are badly named, as isptrarr. The for-loop should define the loop variable and should loop up tobut not including
j: for (int i = 0; i < j; ++i). And the printout"Sentence # contains hidden string prefix length #" is printed even if the
sentence doesn't contain the hidden string. This make me think that the
getHiddenString function is telling us the number of consecutive chars fromthe hidden string contained in the sentence, or some such. In that case it
still doesn't work and is badly named (eg if I give it "two twa twe" and the
hidden string "twe" it says, "Sentence 0 contains hidden string prefix length 4"
Code Snippets
int ch;
while((ch = getchar()) != EOF) {
if (ch == '\n') {
break;
}
// do the necessary stuff
}Context
StackExchange Code Review Q#26567, answer score: 3
Revisions (0)
No revisions yet.