HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Finding palindromes and reverse strings in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reversepalindromesfindingandstrings

Problem

I've created a program that will find if a word is a palindrome or a reverse string (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):

  • You declared the function helpPage(void) to return void *, but never return anything. You probably just meant to write void. Also I would've replaced helpPage with printHelpPage.



-
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 using perror, but the function should return right after printing the error.



  • Memory leak in checkEpi. getline will allocate memory for line and you are responsible for freeing it later on.



  • I don't see a return statement in checkEpi beside from the return true. Explicitly return false!



  • checkPali shouldn't expect the original and the reversed string. The function should check if a single string is a palindrome (the reversing happens inside checkPali) - same thing in checkAll.



-
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 of checkPali and checkEpi to variables named and paliRes and epiRes and later on checks them with paliRes == true. Consider renaming these variables to isPalindrome and isEmordnilap and using them like if(isPalindrome) ..., etc.



  • Minor: I probably would've defined testStrings and defaultWordList at 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.