I'm trying to create a C function to return a pointer to the char with the greatest ascii value in a string, but my function is returning 'H' instead of 'o'. I think it's something to do with the if statement in the for loop but I'm not sure what the problem is. Any help is appreciated. Thanks!

char * select_max(char str[]);

int main(void) {
    printf("%c\n",select_max("Hello"));

    printf("All tests passed successfully.\n");
}

char *select_max(char str[]){
    int length = strlen(str);
    if(length<1){//returns 0 if string length is less than one
        printf("Invalid string.\n");
        return 0;
    }
    char *max = str;

    for(int i=0;i<length;i  ){
        if(str[i] > max){
            max = str[i];
        }
    }
    return *max;
}

CodePudding user response：

Try adding a printout so that you can see your bug in action. For example, just before your if:

printf("%c > %c = %d\n", str[i], max, (str[i] > max));

I believe this will quickly reveal the bug. [Hint: there are 5 bugs in your program.]

CodePudding user response：

Compiler is nice enough to give you quite useful warnings, you should follow them, read carefully and try to understand. It won't point out your logical errors but you can deduce these too in this case.

main.c: In function 'main':
main.c:4:5: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
     printf("%c\n",select_max("Hello"));
     ^~~~~~
main.c:4:5: warning: incompatible implicit declaration of built-in function 'printf'
main.c:4:5: note: include '<stdio.h>' or provide a declaration of 'printf'

main.c: In function 'select_max':
main.c:10:18: warning: implicit declaration of function 'strlen' [-Wimplicit-function-declaration]
     int length = strlen(str);
                  ^~~~~~
main.c:10:18: warning: incompatible implicit declaration of built-in function 'strlen'
main.c:10:18: note: include '<string.h>' or provide a declaration of 'strlen'

You first need to include the libraries that you use functions from, printf is from stdio.h and strlen is from string.h.

main.c:22:12: warning: returning 'char' from a function with return type 'char *' makes pointer from integer without a cast [-Wint-conversion]
     return *max;
            ^~~~

You appear to be confusing chars with char*s. Since select_max is logically supposed to return just a character, (and you are already returning a char) declaring it as below will suffice.

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

char select_max( char str[] );

main.c:19:17: warning: assignment to 'char *' from 'char' makes pointer from integer without a cast [-Wint-conversion]
             max = str[i];

And in the implementation of select_max, there is a similar problem. The temporary variable to hold the highest character is of type char* where it just needs to be a char:

char select_max(char str[]){
//  ^___

    int length = strlen(str);
    if(length<1){//returns 0 if string length is less than one
        printf("Invalid string.\n");
        return 0;
    }

    // Initialize it to 0, has to be lower than any ASCII letters in order your algorithm to work. DO NOT leave it uninitialized.
    char max = 0; // <---

    for(int i=0;i<length;i  ){
        if(str[i] > max){
            max = str[i];
        }
    }
    return max;
}

The code above defines max as a char and initializes it to 0. Assigning str to it was pointless anyway. You are iterating through your string one character at a time and storing the character in a temporary container.

Note that the code in the for loop had already written as if it was of type char.

The same algorithm of course can be implemented in various ways but I suppose this was what you were trying to do.