2
\$\begingroup\$

I was wondering if there are any significant improvements that could be made in order to make the program more concise or efficient, particularly in the findmatch function. However there is a catch: I cannot use strings or array notation; working only with chars and pointer arithmetic.

In short, the program accepts two user inputs: one for the text, and the other for a pattern. Also, the user has a choice of putting "?" chars as "wild" chars that can be treated as any char.The program finds the start char of the pattern in the text input and prints out the remaining chars in the text after the pattern.

#include <stdio.h>

int readline(char* text, int n);
char* findmatch(char* pattern, char* text, int pl, int tl);
void printmessage(char* position, char* text, int pl, int tl); 

int main() {
    char text[40], pattern[40], *position;
    int textlength, patternlength;

    printf("Enter text: ");
    textlength = readline(text, 40);
    printf("Enter pattern: ");
    patternlength = readline(pattern, 40);
    position = findmatch(pattern, text, patternlength, textlength);
    printmessage(position, text, patternlength, textlength);
}

int readline(char* text, int n) {
    int i=0;
    for(i;i<n;i++){
        int ic = getchar();
        if(ic == 10)
            break;
        else{
            *(text+i) = ic;
        }               
     };
     return i;
}

char* findmatch(char* pattern, char* text, int pl, int tl) {
    int indexp = 0;
    char* position = NULL;
    int x=0;
    for(x; x<tl; x++){
        int pc = *(pattern+indexp);
        int tc = *(text+x);
        if(pc == 63 || pc == tc){
            indexp++;
            if(indexp == pl){
                position = (text+x)-indexp+1;
                break;
            }
            printf("status: pass\n");       
        }else{
            indexp = 0;
            printf("status: fail\n");   
        }
    }
    return position;    
}

void printmessage(char* position, char* text, int pl, int tl) {
    if(position == NULL){
        printf("no match\n");
    }else{
        int p = (position-text)+1;
        printf("The pattern was found at char %d. The remaining text chars are: ", p);
        int i = p+pl-1;
        for(i; i<tl; i++){
            printf("%c", *(text+i));
        }
        printf("\n");
    }
 }
\$\endgroup\$
  • \$\begingroup\$ What's the logic behind the ban on strings and array notation? It shouldn't change your program substantially (other than making it harder to read), I'm just curious. \$\endgroup\$ – David Morris Feb 12 '16 at 22:46
  • \$\begingroup\$ Well this program is an assignment for a course that deals with memory and since we are learning about stack frames and things of the nature our professor wants us to know the ins and outs of what we can do with memory addresses and pointers, and how array notation could be substituted with pointer arithmetic. That, and we haven't gotten into strings in C yet. \$\endgroup\$ – Nick Borisenko Feb 12 '16 at 22:53
  • \$\begingroup\$ readline() is a well known system function, here is how that function is part of the code: #include <stdio.h> #include <readline/readline.h> #include <readline/history.h> char * readline (const char *prompt); Suggest using a different name for your function, like: myReadLine() \$\endgroup\$ – user3629249 Feb 12 '16 at 23:01
  • \$\begingroup\$ the posted code contains several 'magic' numbers, (10,40,63). 'magic' numbers make the code much more difficult to understand, debug, maintain. Suggest using #define statements or anenum to give those 'magic' numbers meaningful names, then using those meaningful names throughout the code. \$\endgroup\$ – user3629249 Feb 12 '16 at 23:07
  • \$\begingroup\$ the readline() function can be easily replaced with well known functions like: fgets( text, sizeof text, stdin); followed by: char *newline; if(NULL != (newline = strchr( text, '\n') ) { *newline = '\0'; } \$\endgroup\$ – user3629249 Feb 12 '16 at 23:12
2
\$\begingroup\$

the following does not address any desired updates to findmatch()

But does address the request for improvements to the program.

improvements include:

  1. no implicit conversion warnings from the compiler
  2. does not use a well known system function name
  3. uses system function: strlen() to find the length of the string
  4. does not produce any compiler warnings
  5. uses fgets() to read whole line in one operation
  6. uses strchr() to eliminate any newline read by fgets()
  7. follows the axiom: only one statement per line and (at most) one variable declaration per statement, resulting in much easier to read/understand code
  8. the 'magic' numbers were either eliminated or replaced with meaningful names

and now, the revised code:

#include <stdio.h>
#include <string.h>

#define MAX_INPUT_LEN (40)

// prototypes
size_t myReadLine (char* text, int n);
char* findmatch(char* pattern, char* text, size_t pl, size_t tl);
void printmessage(char* position, char* text, size_t pl, size_t tl);

int main( void )
{
    char text[ MAX_INPUT_LEN ];
    char pattern[ MAX_INPUT_LEN ];
    char *position;
    size_t textlength;
    size_t patternlength;

    printf("Enter text: ");
    textlength = myReadLine( text, (int)(sizeof text) );

    printf("Enter pattern: ");
    patternlength = myReadLine( pattern, sizeof pattern );

    position = findmatch(pattern, text, patternlength, textlength);
    printmessage(position, text, patternlength, textlength);
}



size_t myReadLine( char* text, int n)
{
    size_t i = 0; // initialize to no chars read

    if( NULL != fgets( text, n, stdin ) )
    {
        // eliminate trailing newline, if any
        char * newline;
        if( NULL != ( newline = strchr( text, '\n' ) ) )
        {
            *newline = '\0';
        }

        i = strlen( text );
    }
    return i;
}



char* findmatch(char* pattern, char* text, size_t pl, size_t tl)
{
    size_t indexp = 0;
    char* position = NULL;
    size_t x=0;

    for( ; x<tl; x++)
    {
        int pc = *(pattern+indexp);
        int tc = *(text+x);

        if(pc == '?' || pc == tc)
        {
            indexp++;
            if(indexp == pl)
            {
                position = (text+x)-indexp+1;
                break;
            }
            printf("status: pass\n");
        }

        else
        {
            indexp = 0;
            printf("status: fail\n");
        }
    }
    return position;
}


void printmessage(char* position, char* text, size_t pl, size_t tl)
{
    if(position == NULL)
    {
        printf("no match\n");
    }

    else
    {
        size_t p = (size_t)(position-text)+1;
        printf("%s: %ld. %s\n",
               "The pattern was found at char",
               p,
               "The remaining text chars are: ");
        size_t i =  p+pl-1;

        for(; i<tl; i++)
        {
            printf("%c", *(text+i));
        }
        printf("\n");
    }
 }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.