#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    3
    Rep Power
    0

    Simple function that finds a shorter string within a longer string


    Hello all,

    I've been working on this simple program that I found as an exercise and I don't know why it's not working. I'm sure there is an easier way to solve this problem, but I really wanted to have my idea work. Any constructive help would be greatly appreciated! thanks! Below is the C code:

    #include <stdio.h>

    int findString (char string1[], char string2[])
    {
    int i, j=0, k, l=0, m, n, Counter=0, position, counterarray[25], karray[25], max1;

    for (i=0; string2[i] != '\0'; ++i)
    ++j;

    for (i=0; string1[i] != '\0';++i)
    ++l;

    for (k=0; (l-j);++k){
    for (i=0; i<j;++i){
    if (string2[i]=string1[i+k]){
    ++ Counter;}
    else
    Counter = 0;
    }
    counterarray[k] = Counter;
    karray[k] = k;
    }

    for (i=0;(l-j)-1;++i)
    if (counterarray[i]>counterarray[i+1]){
    counterarray[i]=max1;
    karray[i] = position;}
    else
    counterarray[i+1]=max1;
    karray[i+1] = position;

    n = position+1;


    if (max1 = j){
    printf("The starting position of the second string within the first string is at %i", n);}
    else
    printf("The second string is not contained within the first one");

    return n;
    }


    int main (void)
    {
    char charstring1[100], charstring2[100];

    printf("Please enter a character string:\n");
    scanf("%s", charstring1);
    printf("Please enter a second character string:\n");
    scanf("%s", charstring2);

    findString(charstring1, charstring2);

    return 0;
    }
  2. #2
  3. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,089
    Rep Power
    2222
    First, what do you mean that it doesn't work? Describe the symptoms! Do not try to make us play guessing games with you!

    Second, use code tags! For example, your code, which was stripped of all formatting, would have looked like this:
    Code:
    #include <stdio.h>
    
    int findString (char string1[], char string2[])
    {
        int i, j=0, k, l=0, m, n, Counter=0, position, counterarray[25], karray[25], max1;
    
        for (i=0; string2[i] != '\0'; ++i)
            ++j;
    
        for (i=0; string1[i] != '\0';++i)
            ++l;
    
    for (k=0; (l-j);++k){
        for (i=0; i<j;++i){
            if (string2[i]=string1[i+k]){
                ++ Counter;}
            else
                Counter = 0;
            }
        counterarray[k] = Counter;
        karray[k] = k;
    }
    
    for (i=0;(l-j)-1;++i)
        if (counterarray[i]>counterarray[i+1]){
            counterarray[i]=max1;
            karray[i] = position;}
        else
            counterarray[i+1]=max1;
            karray[i+1] = position;
    
    n = position+1;
    
    
        if (max1 = j){
            printf("The starting position of the second string within the first string is at %i", n);}
        else
            printf("The second string is not contained within the first one");
    
    return n;
    }
    
    
    int main (void)
    {
        char charstring1[100], charstring2[100];
    
        printf("Please enter a character string:\n");
        scanf("%s", charstring1);
        printf("Please enter a second character string:\n");
        scanf("%s", charstring2);
    
        findString(charstring1, charstring2);
    
        return 0;
    }
    Which really is crappy formatting! Especially considering your absolutely idiotic practice of hiding all the braces at the ends of the lines! You're making your code so unreadable that even you aren't able to understand it. If you had a misplaced or missing brace, how could you ever find it? Stupid! Stupid! Stupid!

    Instead, use the Allman Style, which not only indents consistently (which you fail to do!) but also places the braces right out in front where you can actually see them.

    Here is how your code looks using Allman style:
    Code:
    #include <stdio.h>
    
    int findString (char string1[], char string2[])
    {
        int i, j=0, k, l=0, m, n, Counter=0, position, counterarray[25], karray[25], max1;
    
        for (i=0; string2[i] != '\0'; ++i)
            ++j;
    
        for (i=0; string1[i] != '\0';++i)
            ++l;
    
        for (k=0; (l-j);++k)
        {
            for (i=0; i<j;++i)
            {
                if (string2[i]=string1[i+k])
                {
                    ++ Counter;
                }
                else
                    Counter = 0;
            }
            counterarray[k] = Counter;
            karray[k] = k;
        }
    
        for (i=0;(l-j)-1;++i)
            if (counterarray[i]>counterarray[i+1])
            {
                counterarray[i]=max1;
                karray[i] = position;
            }
            else
                counterarray[i+1]=max1;
        karray[i+1] = position;
    
        n = position+1;
    
    
        if (max1 = j)
        {
            printf("The starting position of the second string within the first string is at %i", n);
        }
        else
            printf("The second string is not contained within the first one");
    
        return n;
    }
    
    
    int main (void)
    {
        char charstring1[100], charstring2[100];
    
        printf("Please enter a character string:\n");
        scanf("%s", charstring1);
        printf("Please enter a second character string:\n");
        scanf("%s", charstring2);
    
        findString(charstring1, charstring2);
    
        return 0;
    }
    See how much more readable it is? Stop handicapping yourself and shooting yourself in the foot!

    Third, why did you try to run that program? You have warnings!:
    C:TEST>gcc -Wall nabla.c
    nabla.c: In function `findString':
    nabla.c:17: warning: suggest parentheses around assignment used as truth value
    nabla.c:41: warning: suggest parentheses around assignment used as truth value
    nabla.c:5: warning: unused variable `m'

    C:TEST>
    Never run a program that gives you warnings! Warnings mean that there's something wrong with your program! Never ignore warnings! Ignoring warnings is just plain stupid, stupid, stupid!

    Here are lines 17 and 41:
    Line 17 -- if (string2[i]=string1[i+k])
    Line 41 -- if (max1 = j)

    Is that really what you intended to do? Assign values? I trust you do realize that an assignment statement evaluates to the value that was assigned, so unless one of those variables was having a zero assigned to it those conditions would always evaluate to true.

    If you instead wanted to test for equality, then you would have needed to use == . Remember, == is for testing for equality, whereas = is for assigning a value. Two entirely different things.

    PS

    Consider this for-loop (properly formatted to see its structure):
    Code:
        for (i=0;(l-j)-1;++i)
            if (counterarray[i]>counterarray[i+1])
            {
                counterarray[i]=max1;
                karray[i] = position;
            }
            else
                counterarray[i+1]=max1;
    
        karray[i+1] = position;
    Why are you changing an element in karray if the condition is true, but not if it's false? And why are you choosing to always change karray[1] after leaving that for-loop?

    Is this what you had intended instead?:

    Code:
        for (i=0;(l-j)-1;++i)
            if (counterarray[i]>counterarray[i+1])
            {
                counterarray[i]=max1;
                karray[i] = position;
            }
            else
            {
                counterarray[i+1]=max1;
                karray[i+1] = position;
            }
    If so, then this is an example of you screwing yourself up with your totally idiotic practice of hiding the braces!
    Last edited by dwise1_aol; July 17th, 2013 at 11:00 AM.
  4. #3
  5. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    3
    Rep Power
    0
    #include <stdio.h>

    void findString (char string1[], char string2[])
    {
    int i, j=0, k, l=0, n, Counter=0, position, counterarray[25], karray[25], max1;

    //saves length of strings 1 and 2

    for (i=0; string2[i] != '\0'; ++i)
    ++j;

    for (i=0; string1[i] != '\0';++i)
    ++l;

    //compares the characters in string 2 to those in string 1 whilst simultaneously moving over 1 character each loop (e.g. the first loop basically compares 'hat' to 'cha' and the second loop compares 'hat' to 'hat'. If the characters are equivalent, a count is added. After each iteration, the value of "Counter" is placed into a separate array for later. A second array called "karray" stores the position values associated with the "Counter" array

    for (k=0; (l-j);++k)
    {
    for (i=0; i<j;++i)
    {
    if (string2[i]==string1[i+k])
    {
    ++ Counter;
    }
    else
    Counter = 0;
    }
    counterarray[k] = Counter;
    karray[k] = k;
    }

    //This section finds the maximum value of the countarray and karray. If the max of the countarray is equivalent to the number of characters in the second string, then we're done.

    for (i=0;(l-j)-1;++i)
    if (counterarray[i]>counterarray[i+1])
    {
    counterarray[i]=max1;
    karray[i] = position;
    }
    else
    {
    counterarray[i+1]=max1;
    karray[i+1] = position;
    }

    n = position+1;


    if (max1 == j)
    {
    printf("The starting position of the second string within the first string is at %i", n);
    }
    else
    printf("The second string is not contained within the first one");
    }


    int main (void)
    {
    char charstring1[100], charstring2[100];

    printf("Please enter a character string:\n");
    scanf("%s", charstring1);
    printf("Please enter a second character string:\n");
    scanf("%s", charstring2);

    findString(charstring1, charstring2);

    return 0;
    }

    Here is the new code. I formatted it correctly this time! :D [Nevermind, for some reason, it's not letting me tab correctly in this editor. When I paste it into the editor, everything's spaced out, but it doesn't last through the posting.]

    Also, the 'warnings' part of my CodeBlocks IDE misses things sometimes, sorry about that. This code shouldn't have any silly mistakes and everything is now visible. As far as I'm concerned, everything I've written seems to make logical sense, but when I run the program, it doesn't work correctly.

    Specifically, I always use "chat" for string 1 and "hat" for string two as a test. Right after hitting enter for the second string, the program does not do anything. Is there something elementary that I'm missing?
  6. #4
  7. Contributing User
    Devshed Supreme Being (6500+ posts)

    Join Date
    Jan 2003
    Location
    USA
    Posts
    7,089
    Rep Power
    2222
    USE CODE TAGS! Without code tags, your code looks completely unformatted!

    [code] <insert your formatted code here> [/code]

    There's also supposed to be some button in the advanced editor, # I think, that does the same thing, but having done a lot of HTML I always just type in the tags.

    Hitting the Reply button displays what was originally entered, including all codes. Apply code tags to your code we get:
    Code:
    #include <stdio.h>
    
    void findString (char string1[], char string2[])
    {
        int i, j=0, k, l=0, n, Counter=0, position, counterarray[25], karray[25], max1;
    
    //saves length of strings 1 and 2
    
        for (i=0; string2[i] != '\0'; ++i)
            ++j;
    
        for (i=0; string1[i] != '\0';++i)
            ++l;
    
    //compares the characters in string 2 to those in string 1 whilst simultaneously moving over 1 character each loop (e.g. the first loop basically compares 'hat' to 'cha' and the second loop compares 'hat' to 'hat'. If the characters are equivalent, a count is added. After each iteration, the value of "Counter" is placed into a separate array for later. A second array called "karray" stores the position values associated with the "Counter" array
    
    for (k=0; (l-j);++k)
              {
        for (i=0; i<j;++i)
                 {
            if (string2[i]==string1[i+k])
                        {
                ++ Counter;
                        }
            else
                Counter = 0;
              }
        counterarray[k] = Counter;
        karray[k] = k;
    }
    
    //This section finds the maximum value of the countarray and karray. If the max of the countarray is equivalent to the number of characters in the second string, then we're done.
    
    for (i=0;(l-j)-1;++i)
        if (counterarray[i]>counterarray[i+1])
                   {
            counterarray[i]=max1;
            karray[i] = position;
                   }
        else
              {
            counterarray[i+1]=max1;
            karray[i+1] = position;
                }
    
    n = position+1;
    
    
        if (max1 == j)
                {
            printf("The starting position of the second string within the first string is at %i", n);
               }
        else
            printf("The second string is not contained within the first one");
    }
    
    
    int main (void)
    {
        char charstring1[100], charstring2[100];
    
        printf("Please enter a character string:\n");
        scanf("%s", charstring1);
        printf("Please enter a second character string:\n");
        scanf("%s", charstring2);
    
        findString(charstring1, charstring2);
    
        return 0;
    }
    Originally Posted by nabls_squared
    Here is the new code. I formatted it correctly this time! :D
    No, you did not. The indentation is still wildly inconsistent. But at least you're no longer hiding the braces.

    Also, this time I get no warnings. To my knowledge, Code::Blocks uses the MinGW gcc compiler, which is the same one I used, albeit very likely different versions.


    Originally Posted by nabla_squared
    As far as I'm concerned, everything I've written seems to make logical sense, but when I run the program, it doesn't work correctly.

    Specifically, I always use "chat" for string 1 and "hat" for string two as a test. Right after hitting enter for the second string, the program does not do anything. Is there something elementary that I'm missing?
    One thing you will learn eventually is that computers do not care at all about your opinion.

    Please direct your attention to Line 20 (in my editor, but that should get you looking in about the right place in your editor), which is this line:
    for (k=0; (l-j);++k)

    if l== j (ie, if both strings are the same length), then (l-j) will be zero and that loop will never be executed. If l != j (ie, if the two strings are of different lengths), then (l-j) will be non-zero and that condition will be true, allowing the loop to be executed.

    Now, how are you going to get out of that loop? The only way out of that loop is for the conditional expression, (l-j) to somehow eventually become zero. When will that happen? Since I see no evidence of either l nor j ever changing inside that for-loop, I would say that it never happens. You will remain inside that loop forever.

    You are trapped inside of an infinite loop. You need to rethink that loop.

    PS
    Regarding for (k=0; (l-j);++k)

    Here's a hint. I learned C back in 1990 and have been working primarily in C and C++ (they both have identical for loops) ever since then. In all that time, I don't think I've ever seen a for-loop conditional expression like that.

    That should raise a red flag for you. That perhaps you should use a more conventional approach, a more conventional and straight-forward conditional expression.
    Last edited by dwise1_aol; July 17th, 2013 at 02:36 PM.
  8. #5
  9. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    3
    Rep Power
    0
    Code:
    #include <stdio.h>
    
    void findString (char string1[], char string2[])
    {
        int i, j=0, k, l=0, n, Counter=0, position, counterarray[100], karray[100], max1;
    
        for (i=0; string2[i] != '\0'; ++i)
            ++j;
    
        for (i=0; string1[i] != '\0';++i)
            ++l;
    
    for (k=0;k<((l-j)+1);++k){
        for (i=0; i<j;++i){
            if (string2[i]==string1[i+k]){
                ++ Counter;}
            else{
                Counter = 0;}
            }
        counterarray[k] = Counter;
        karray[k] = k;
    }
    
    if (l!=j){
    for (i=0;i<((l-j)+1);++i)
        if (counterarray[i]==j){
            max1=counterarray[i];
            position=karray[i];}
    }
    else {
        max1=j;
        position=0;
    }
    n = position+1;
    
        if (n>=0){
            printf("The starting position of the second string within the first string is at %i.", n);}
        else{
            printf("The second string is not contained within the first one.");}
    }
    
    int main (void)
    {
        char charstring1[100], charstring2[100];
    
        printf("Please enter a character string:\n");
        scanf("%s", charstring1);
        printf("Please enter a second character string:\n");
        scanf("%s", charstring2);
    
        findString(charstring1, charstring2);
    
        return 0;
    }
    I GOT IT!

    Thank you for your input though! I'm new at this, so I was unaware about the conditional argument stuff.

IMN logo majestic logo threadwatch logo seochat tools logo