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

Returning the length of a 'hidden' string within each of the sentences of another string

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

This function has many issues, some minor, such as casting malloc, using
sizeof(char) (sizeof char is 1 by definition) and the arbitrary limit on a
new 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 realloc
call rather than computing it twice. And your realloc call as it currently
stands leaks memory on failure - the original string str1 is not freed by
realloc on failure.

In function divideStringIntoSentences

You need to pass the size of sentences[] into the function to allow for overflow
detection.

In function getHiddenString

Parameter names of sentence and hidden might be more understandable than
str1, str2. More seriously, I don't know what it is supposed to
do. 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 main

Although the functions handle malloc failures, main does not check for
str1 in the returned values. Again str1 and str2 are badly named, as is
ptrarr. The for-loop should define the loop variable and should loop up to
but 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 from
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"

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.