patterncMinor
Finding palindromes and reverse strings in C
Viewed 0 times
reversepalindromesfindingandstrings
Problem
I've created a program that will find if a word is a palindrome or a reverse string (
How it works:
The palindrome part works by reversing the string, and comparing the two strings using
How it checks if the word is a reverse string is by running through a wordlist line by line, stripping the string using a strip function and testing if the word is in the wordlist. Examples of usage:
`e@ubuntu:~/bin/c$ ./epi.exe -h
usage: epi -h -t -s [string] [file]
-h Print this help and exit
-t Run the test strings.
-s Input a string to check if it is either a palindrome or
a emordinlap, followed by the path to a file.
e@ubuntu:~/bin/c$ ./epi.exe -t
'a' is a palindrome, it is the same forward and backwards
Reverse of 'a' is a emordnilap, it spells a word backwards.
'ab' is not a palindrome, it is not the same forward and backwards
Reverse of 'ab' is not a emordnilap, it does not spell a word backwards
'abc' is not a palindrome, it is not the same forward and backwards
Reverse of 'abc' is not a emordnilap, it does not spell a word backwards
'another' is not a palindrome, it is not the same forward and backwards
Reverse of 'another' is not a emordnilap, it does not spell a word backwards
'cbc' is a palindrome, it is the same forward and backwards
Reverse of 'cbc' is not a emordnilap, it does not spell a word backwards
'|0|' is a palindrome, it is the same forward and backwards
Reverse of '|0|' is not a emordnilap, it does not spell a word backwards
'palindrome' is not a palindrome, it is not the same forward and backwards
Reverse of 'palindrome' is not a emordnilap, it does not spell a word backwards // technically emordnilap is not a word
e@ubuntu:~/bin/c$ ./epi.exe -s
You must provide a string to test.
usage: epi -h -t -s [string] [file]
-h Print this help and exit
-t Run the test strings.
-s
emordnilap). How it works:
The palindrome part works by reversing the string, and comparing the two strings using
strcmp to see if they match one another. If they do match, it will return true, if they do not match false. How it checks if the word is a reverse string is by running through a wordlist line by line, stripping the string using a strip function and testing if the word is in the wordlist. Examples of usage:
`e@ubuntu:~/bin/c$ ./epi.exe -h
usage: epi -h -t -s [string] [file]
-h Print this help and exit
-t Run the test strings.
-s Input a string to check if it is either a palindrome or
a emordinlap, followed by the path to a file.
e@ubuntu:~/bin/c$ ./epi.exe -t
'a' is a palindrome, it is the same forward and backwards
Reverse of 'a' is a emordnilap, it spells a word backwards.
'ab' is not a palindrome, it is not the same forward and backwards
Reverse of 'ab' is not a emordnilap, it does not spell a word backwards
'abc' is not a palindrome, it is not the same forward and backwards
Reverse of 'abc' is not a emordnilap, it does not spell a word backwards
'another' is not a palindrome, it is not the same forward and backwards
Reverse of 'another' is not a emordnilap, it does not spell a word backwards
'cbc' is a palindrome, it is the same forward and backwards
Reverse of 'cbc' is not a emordnilap, it does not spell a word backwards
'|0|' is a palindrome, it is the same forward and backwards
Reverse of '|0|' is not a emordnilap, it does not spell a word backwards
'palindrome' is not a palindrome, it is not the same forward and backwards
Reverse of 'palindrome' is not a emordnilap, it does not spell a word backwards // technically emordnilap is not a word
e@ubuntu:~/bin/c$ ./epi.exe -s
You must provide a string to test.
usage: epi -h -t -s [string] [file]
-h Print this help and exit
-t Run the test strings.
-s
Solution
First off: I really like reading your code. The code style looks clean and the intend of all variables seem clear to the reader - good naming. This was pretty quick and I may revisit this post later - don't have enough time right now.
Here are some things I noticed (in no particular order):
-
Declare variables when you first use them. This makes the intend more clear to the reader. Example in
I think this makes the use of each variable easier to understand, otherwise
the reader could be confused (What do I need all these variables for?).
-
I'm not sure if creating an extra function
-
Pay attention to
Same with
-
Take a look at your current
but instead write
This is what I came up with:
(The code should just give you a rough example on how to do it. I'm currently in a rush, so please excuse any mistakes made)
```
#include
#include
#include
static void printHelpPage(void) {
puts("Usage: epi -h -t -s [string] [file]\n");
puts("-h Print this help and exit\n");
puts("-t Run the test strings.\n");
puts("-s Input a string to check if it is either a palindrome or");
puts(" a emordnilap, followed by the path to a file.");
}
// In-place reverse null-terminated string
static char reverse(char str) {
size_t len;
if (str != NULL && (len = strlen(str)) > 1) {
char src, dest;
src = str;
dest = src + len - 1;
while (src 0) {
if (strcmp(strtok(line, "\n"), reversed) == 0) {
return 1;
}
}
fclose(wordListFile);
free(reversed);
return 0;
}
// Return -1 on error, 0 on no palindrome, 1 on palindrome
static int checkPalidrome(const char *str) {
char *reversed = malloc(strlen(str) + 1);
if (!reversed) {
perror("Failed to allocate memory to store reversed string.\n");
return -1;
}
strcpy(reversed, str);
reverse(reversed);
int result = strcmp(str, reversed) == 0;
free(reversed);
return result;
}
static void printStatus(const char origin, const char filePath) {
int isPalindrome, isEmordnilap;
isPalindrome = checkPalidrome(origin);
isEmordnilap = checkEmordnilap(origin, filePath);
if (isPalindrome == -1 || isEmordnilap == -1) {
// We already see the error message printed from checkPalindrome/checkEmordnilap.
return;
}
if (isPalindrome) {
printf("\n'%s' is a palindrome, it is the same forward and backwards\n", origin);
} else {
printf("\n'%s' is not a palindrome, it is not the same forward and backwards\n", origin);
}
if (isEmordnilap) {
printf("Reverse of '%s' is a emordnilap, it spells a word backwards.\n\n", origin);
} else {
printf("Reverse of '%s' is not a emordnilap, it does
Here are some things I noticed (in no particular order):
- You declared the function
helpPage(void)to returnvoid *, but never return anything. You probably just meant to writevoid. Also I would've replacedhelpPagewithprintHelpPage.
-
Declare variables when you first use them. This makes the intend more clear to the reader. Example in
reverse(char *str):// Reverse a given null-terminated string
static char *reverse(char *str)
{
if (str != NULL)
{
size_t len = strlen(str);
if (len > 1)
{
char *src, *dst;
src = str;
dst = src + len - 1;
while (src < dst)
{
char tmp = *src;
*src++ = *dst;
*dst-- = tmp;
}
}
}
return str;
}I think this makes the use of each variable easier to understand, otherwise
the reader could be confused (What do I need all these variables for?).
-
I'm not sure if creating an extra function
strip(char *s) for a single strtok call is really necessary. -
Pay attention to
const correctness in checkEpi: Mark reversed and filePath const, since checkEpi doesn't modify them through these pointers:static bool checkEpi(const char *reversed, const char *filePath);Same with
checkPali / checkAll.- Handle failed
fopen. You do print and error message usingperror, but the function should return right after printing the error.
- Memory leak in
checkEpi.getlinewill allocate memory forlineand you are responsible for freeing it later on.
- I don't see a
returnstatement incheckEpibeside from thereturn true. Explicitly returnfalse!
checkPalishouldn't expect the original and the reversed string. The function should check if a single string is a palindrome (the reversing happens insidecheckPali) - same thing incheckAll.
-
Take a look at your current
checkPali implementation. Never write code like this: if (something) {
return true;
} else {
return false;
}but instead write
return something; right away . checkAll: Your current code assigns the return values ofcheckPaliandcheckEpito variables named andpaliResandepiResand later on checks them withpaliRes == true. Consider renaming these variables toisPalindromeandisEmordnilapand using them likeif(isPalindrome) ..., etc.
- Minor: I probably would've defined
testStringsanddefaultWordListat the top of the file via the#define-directive.
- Minor: You accidentally put one semicolon too many into
helpPage(void).
This is what I came up with:
(The code should just give you a rough example on how to do it. I'm currently in a rush, so please excuse any mistakes made)
```
#include
#include
#include
static void printHelpPage(void) {
puts("Usage: epi -h -t -s [string] [file]\n");
puts("-h Print this help and exit\n");
puts("-t Run the test strings.\n");
puts("-s Input a string to check if it is either a palindrome or");
puts(" a emordnilap, followed by the path to a file.");
}
// In-place reverse null-terminated string
static char reverse(char str) {
size_t len;
if (str != NULL && (len = strlen(str)) > 1) {
char src, dest;
src = str;
dest = src + len - 1;
while (src 0) {
if (strcmp(strtok(line, "\n"), reversed) == 0) {
return 1;
}
}
fclose(wordListFile);
free(reversed);
return 0;
}
// Return -1 on error, 0 on no palindrome, 1 on palindrome
static int checkPalidrome(const char *str) {
char *reversed = malloc(strlen(str) + 1);
if (!reversed) {
perror("Failed to allocate memory to store reversed string.\n");
return -1;
}
strcpy(reversed, str);
reverse(reversed);
int result = strcmp(str, reversed) == 0;
free(reversed);
return result;
}
static void printStatus(const char origin, const char filePath) {
int isPalindrome, isEmordnilap;
isPalindrome = checkPalidrome(origin);
isEmordnilap = checkEmordnilap(origin, filePath);
if (isPalindrome == -1 || isEmordnilap == -1) {
// We already see the error message printed from checkPalindrome/checkEmordnilap.
return;
}
if (isPalindrome) {
printf("\n'%s' is a palindrome, it is the same forward and backwards\n", origin);
} else {
printf("\n'%s' is not a palindrome, it is not the same forward and backwards\n", origin);
}
if (isEmordnilap) {
printf("Reverse of '%s' is a emordnilap, it spells a word backwards.\n\n", origin);
} else {
printf("Reverse of '%s' is not a emordnilap, it does
Code Snippets
// Reverse a given null-terminated string
static char *reverse(char *str)
{
if (str != NULL)
{
size_t len = strlen(str);
if (len > 1)
{
char *src, *dst;
src = str;
dst = src + len - 1;
while (src < dst)
{
char tmp = *src;
*src++ = *dst;
*dst-- = tmp;
}
}
}
return str;
}static bool checkEpi(const char *reversed, const char *filePath);if (something) {
return true;
} else {
return false;
}#include <stdio.h>
#include <stdlib.h>
#include <string.h>
static void printHelpPage(void) {
puts("Usage: epi -h -t -s [string] [file]\n");
puts("-h Print this help and exit\n");
puts("-t Run the test strings.\n");
puts("-s Input a string to check if it is either a palindrome or");
puts(" a emordnilap, followed by the path to a file.");
}
// In-place reverse null-terminated string
static char *reverse(char *str) {
size_t len;
if (str != NULL && (len = strlen(str)) > 1) {
char *src, *dest;
src = str;
dest = src + len - 1;
while (src < dest) {
char tmp = *src;
*src++ = *dest;
*dest-- = tmp;
}
}
return str;
}
// Return values: -1 on error, 0 for no Emordnilap, 1 for Emordnilap
static int checkEmordnilap(const char *str, const char *filePath) {
char *reversed = malloc(strlen(str) + 1);
if (!reversed) {
perror("Failed to allocate memory to store reversed string ");
return -1;
}
strcpy(reversed, str);
reverse(reversed);
FILE *wordListFile = fopen(filePath, "r");
if (!wordListFile) {
free(reversed);
perror("Failed to open wordList ");
return -1;
}
char *line = NULL;
size_t linecap = 0;
ssize_t linelen;
while((linelen = getline(&line, &linecap, wordListFile)) > 0) {
if (strcmp(strtok(line, "\n"), reversed) == 0) {
return 1;
}
}
fclose(wordListFile);
free(reversed);
return 0;
}
// Return -1 on error, 0 on no palindrome, 1 on palindrome
static int checkPalidrome(const char *str) {
char *reversed = malloc(strlen(str) + 1);
if (!reversed) {
perror("Failed to allocate memory to store reversed string.\n");
return -1;
}
strcpy(reversed, str);
reverse(reversed);
int result = strcmp(str, reversed) == 0;
free(reversed);
return result;
}
static void printStatus(const char *origin, const char *filePath) {
int isPalindrome, isEmordnilap;
isPalindrome = checkPalidrome(origin);
isEmordnilap = checkEmordnilap(origin, filePath);
if (isPalindrome == -1 || isEmordnilap == -1) {
// We already see the error message printed from checkPalindrome/checkEmordnilap.
return;
}
if (isPalindrome) {
printf("\n'%s' is a palindrome, it is the same forward and backwards\n", origin);
} else {
printf("\n'%s' is not a palindrome, it is not the same forward and backwards\n", origin);
}
if (isEmordnilap) {
printf("Reverse of '%s' is a emordnilap, it spells a word backwards.\n\n", origin);
} else {
printf("Reverse of '%s' is not a emordnilap, it does not spell a word backwards\n\n", origin);
}
}
#define TEST_STRINGS { "a", "ab", "abc", "another", "|0|", "palindrome" }
#define DEFAULT_WORD_LIST "/usr/share/dict/propernames"
int main(int argc, char *argv[]) {
if (argc < 2) {Context
StackExchange Code Review Q#159915, answer score: 4
Revisions (0)
No revisions yet.