1
\$\begingroup\$

This is a simple attempt by me to test whether two words are anagrams or not:

#include <stdio.h>
#include <ctype.h>

#define CH_LEN 15
#define N 26

int main(void) {
    char track_letters[N] = {0};
    char first_word[CH_LEN];
    char sec_word[CH_LEN];
    printf("Enter first word: ");
    int i;
    for(i = 0; i < CH_LEN; i++) {
        first_word[i] = getchar();
        if(first_word[i] == '\n')
            break;
        if(!isalpha(first_word[i]))
            --i;

        track_letters[(tolower(first_word[i]) - 'a')]++;


    }
    printf("Enter second word: ");
    for(i = 0; i < CH_LEN; i++) {
        sec_word[i] = getchar();
        if(sec_word[i] == '\n')
            break;
        if(!isalpha(sec_word[i]))
            --i;
        track_letters[(tolower(sec_word[i]) - 'a')]--;

    }
    for(i = 0; i < CH_LEN; i++) {
        if(track_letters[i] != 0) {
            printf("The words are not anagrams");
            return 0;
        }
    }
    printf("The words are anagrams");

    return 0;
}

You can use any method of C (structure, pointers, functions etc.).

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

You limit yourself to anagrams of 15 letters.

#define CH_LEN 15
char first_word[CH_LEN];
char sec_word[CH_LEN];

But you don't actually use words after they are input.
You can fix the limitation (and a potentially serious bug) by not using the concept of a line. Just read characters.

In the code just read characters:

char nextInput = getchar();

No all alphabets are 26 letters.

#define N 26

I would have done this for clarity to show that you are looking at a-z:

#define N  ('z' - 'a' + 1)

Also N is not that descriptive. Personally I would have preferred a global static variable.

static const AlphabetSize = ('z' - 'a' + 1);

You basically repeat the same piece of code twice. This violates the DRY principle. Remove this code into a function and call the function passing parameters for the differences.

    int i;
    for(i = 0; i < CH_LEN; i++) {
        first_word[i] = getchar();
        if(first_word[i] == '\n')
            break;
        if(!isalpha(first_word[i]))
            --i;

        track_letters[(tolower(first_word[i]) - 'a')]++;


    }
    printf("Enter second word: ");
    for(i = 0; i < CH_LEN; i++) {
        sec_word[i] = getchar();
        if(sec_word[i] == '\n')
            break;
        if(!isalpha(sec_word[i]))
            --i;
        track_letters[(tolower(sec_word[i]) - 'a')]--;

    }

I would write like this:

    ProcessesAnagram(track_letters, +1);  // Reads first Line
    ProcessesAnagram(track_letters, -1);  // Reads second line

Since we are not longer bound by a word size. I would change the loop to check for the end of line (or end of input)).

 void ProcessesAnagram(int* track_letters, int increment)
 {
    int nextLetter = getchar();
    for(;nextLetter != EOF && nextLetter != '\n'; nextLetter = getchar())
    {
        if(!isalpha(nextLetter)
        {    continue;           // If not a letter just start the next iteration
        }

        // Note: increment is +1 first anagram
        //       increment is -1 second anagram (so they will cancel out)
        track_letters[(tolower(nextLetter) - 'a')] += increment;    
    }
 }

There is a bug here

You are using the wrong size: CH_LEN is the size of the words. But the track_letters array has a size of N.

    for(i = 0; i < CH_LEN; i++) {
        if(track_letters[i] != 0) {
            printf("The words are not anagrams");
            return 0;
        }
    }
\$\endgroup\$
2
  • \$\begingroup\$ In a non-26-char alphabet, the extra chars are not going to be between 'a' and 'z', are they? Also, the continue is not really necessary; just reverse the condition. \$\endgroup\$ Mar 23, 2013 at 21:08
  • \$\begingroup\$ @WilliamMorris: Both good points. But I don't mind the continue. In my opinion (and this is totally depended on context and how your team feels so can be very variable) I think the error test and continue helps show state that are not being considered thus makes the code more readable. \$\endgroup\$ Mar 23, 2013 at 21:17
0
\$\begingroup\$

Adding to what Loki said, I would have taken the two words from the command line and I would split the checking code into a separate function:

int main(int argc, char **argv)
{
    int track_letters[N] = {0};
    if (argc != 3) {
        fprintf(stderr, "Usage: %s word1 word2\n", argv[0]);
    } else {
        processes_anagram(track_letters, argv[1], 1);
        processes_anagram(track_letters, argv[2], -1);
        check_letters(track_letters);
    }
    return 0;
}

Also add a \n onto the printf strings

\$\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.