Reputation:
I am writing a program to compare different strings. Specifically chemical elements that end with an OH
. I have to return -1 if the string ends with OH
. However, my program doesn't work. Where am I wrong?
#include <stdio.h>
#include <string.h>
int hydroxide(char *string);
int main() {
char *string;
printf("Enter String:");
gets(string);
printf("%d", hydroxide(string));
}
int hydroxide(char *string) {
string = strrchr(string, 'O');
if (string != NULL)
return (strcmp(string, "OH"));
return (-1);
}
Upvotes: 3
Views: 637
Reputation: 144820
Your function is too complicated and it returns 0
if the suffix is OH
. A simpler way is to compute the length of the string and if this length is at least 2, compare the last 2 characters with 'O'
and 'H'
:
int hydroxide(const char *string) {
size_t len = strlen(string);
if (len >= 2 && string[len - 2] == 'O' && string[len - 1] == 'H')
return -1;
else
return 0;
}
Furthermore, the main
function has undefined behavior: string
is an uninitialized char
pointer: passing it to gets()
will cause undefined behavior when gets()
tries to write bytes to it. Note also that gets()
is obsolete and has been removed from the latest version of the C Standard because there is no way to prevent a buffer overflow for sufficiently long input strings. Use fgets()
instead and remove the trailing newline, if any:
Here is a modified version:
#include <stdio.h>
#include <string.h>
int hydroxide(const char *string) {
size_t len = strlen(string);
if (len >= 2 && string[len - 2] == 'O' && string[len - 1] == 'H')
return -1;
else
return 0;
}
int main() {
char buf[80];
printf("Enter String: ");
if (fgets(buf, sizeof buf, stdin)) {
buf[strcspn(buf, "\n")] = '\0'; // strip the newline if any
printf("%d\n", hydroxide(string));
}
return 0;
}
Upvotes: 1
Reputation: 311038
For starters the logic of the function is wrong.
Usually such a function should return 1
(or a positive value) that corresponds to the logical true
or 0
that corresponds to the logical false
when it answers to a question like "yes or no".
This call
strcmp(string, "OH")
returns 0 if two strings are equal. Otherwise the function can return any positive or negative value depending on whether the first string is greater or less than the second string.
Apart from this the function parameter should have the qualifier const
because the passed string is not changed within the function.
You did not reserve a memory where you are going to read a string. The declared pointer
char *string;
is uninitialized and has an indeterminate value. Thus this call
gets(string);
invokes undefined behavior.
Take into account that the function gets
is an unsafe function and is not supported by the C Standard. Instead you should use the standard C function fgets
.
And it will be much better if the function will be more generic. That is when it can check any supplied suffix of a string. Always try to write more generic functions. In this case they can be reusable.
Below there is a demonstrative program that shows how the function can be defined.
#include <stdio.h>
#include <string.h>
int hydroxide( const char *s, const char *suffix )
{
size_t n1 = strlen( s );
size_t n2 = strlen( suffix );
return !( n1 < n2 ) && strcmp( s + n1 - n2, suffix ) == 0;
}
int main(void)
{
enum { N = 100 };
char s[N];
while ( 1 )
{
printf( "Enter a String (empty string - exit): " );
if ( fgets( s, N, stdin ) == NULL || s[0] == '\n' ) break;
s[ strcspn( s, "\n" ) ] = '\0';
printf( "%s\n", hydroxide( s, "OH" ) ? "true" : "false" );
}
return 0;
}
The program output might look like
Enter a String (empty string - exit): brogrammerOH
true
Enter a String (empty string - exit):
Upvotes: 1
Reputation:
Get the length of the string and check the last two characters.
int len = strlen(string);
if(string[len-1] == 'H' && string[len-2] =='O')
return -1;
Upvotes: 0