Faulty anagram function

[noobstudent]

Hey everyone! Happy new year!

I've been trying to complete an exercise where the program must take two words and answer whether or not the words are anagrams of each other.

I was attempting to do it with one function called 'checkAnagram' which was supposed to go through each element of the first word and compare it to each element of the second word until a match is found, then repeat the process for each element of the first until the variable 'found' is either 'true' or 'false'. I then realised that this function was too simplistic and wouldn't consider the number of each letter, the number of characters, etc. and therefore wouldn't give me the correct response anyway.

I now have the teacher's solutions for the exercises which I glanced quickly and saw he used 'for' loops to get the various little tasks done like counting the number of characters, and then checking whether each word contains the same characters, so I will try writing the program that way.

However, I am still curious about why my code doesn't give the expected response. When I run the program, it simply gives me 'NO', no matter what words I input. Can anyone point out to me my error?

#include <iostream>
using namespace std;

bool checkAnagram(char word1[], char word2[]);

int main()
{
    char word1[100];
    char word2[100];
    bool result;
    
        cout << "Is it an anagram?\n";
        cout << "First word: ";
        cin >> word1;
        cout << "Second word: ";
        cin >> word2;
        
        result = checkAnagram(word1, word2);
        
        if(result==true)
        {
            cout << "YES\n";
        }
        else
        {
            cout << "NO\n";
        }
    
    return 0;
}

bool checkAnagram(char word1[], char word2[])
{
    int pos1, pos2;
    bool found;
    char element;
    
    pos1 = 0;
    
    do
    {
        element = word1[pos1];
        pos2 = 0;
    
            do
            {
                if(word2[pos2] == element)
                {
                    found = true;
                }
                else
                {
                    ++pos2;
                    found = false;
                }
            }
            while ((pos2 < 100) && (found == false));
            
        ++pos1;
    }
    while ((pos1 < 100) && (found == true));
    
    return found;
}

[seeplus]

L35 - init found to false
L54 - remove (work out why!)

Is it an anagram?
First word: qwerty
Second word: ertyq
YES

Is it an anagram?
First word: qwerrt
Second word: asdfg
NO

[dutch]

@seeplus, try "ab" and "ac". ;-)

@noobstudent, The problem is that you don't stop when you hit the actual end of the input strings. It's still wrong, of course, but you know that.

bool checkAnagram(char word1[], char word2[])
{
    int len1 = strlen(word1);
    int len2 = strlen(word2);

    if (len1 != len2)
        return false;

    bool found = false;
    int pos1 = 0;
    do
    {
        int pos2 = 0;
        do
        {
            found = (word2[pos2] == word1[pos1]);
            ++pos2;
        }
        while (pos2 < len2 && !found);
        ++pos1;
    }
    while (pos1 < len1 && found);
    
    return found;
}

An example of something this still gets wrong is "aaaab" and "abbbb".

[jonnin]

if you want a pure anagram (that is, rearrange word one to get word two, using every letter of word one only once) then you can just sort word 1 and word 2 and compare that. If it is the same, they are anagrams. Beyond that, where you start asking if word 2 is IN word one but may not use all the letters of one, it takes more logic.

[noobstudent]

@seeplus, thanks for the idea! @dutch does make a good point re: trying "ab" and "ac", though! Blame it on my Frankencode, lol!

@dutch; The thing that confuses me a bit here is why should it make a difference whether it stops at the end of the actual string or whether it stops at the limit of the array? For example, if you have two words of 4 characters, there would be the same number of 'slots' used in the array, as well as the null terminator and the vacant slots, so how come it doesn't 'read' that? Is that simply the way it is, or is there a more nuanced answer? (Sorry if this question makes little sense, it's been a fairly long day for me...haha).

@jonnin, That's a cool workaround that I hadn't seen before. I'm not sure if it could be used along with the 'tolower' function to abide by the exercise asking for the anagram solver to ignore whether the characters are uppercase, but I'm definitely going to give it a go, too.

I stumbled upon another little issue - when I try to compare two string lengths, I'm getting bool = 0 despite the two strings being the same length...???

#include <iostream>
#include <cstring>
using namespace std;

bool checkLength(char frase1[], char frase2[]);

int main()
{
    char word1[100] = "shi";
    char word2[100] = "poo";
    bool samelength;
    
    checkLength(word1, word2);
    cout << samelength;
    
    return 0;
}

bool checkLength(char word1[], char word2[])
{
    int len1, len2;
    bool samelength;
    
    len1 = strlen(frase1);
    len2 = strlen(frase2);
    
    if(len1 == len2)
    {
        samelength = true;
    }
    else
    {
        samelength = false;
    }
       
    return samelength;
}

[George P]

I'm getting bool = 0 despite the two strings being the same length...???

You aren't saving the return value of checkLength(). You are displaying an uninitialized sameLength variable in line 14. You should bool sameLength = checkLength(word1, word2);.

[jonnin]

to tolower an entire string, you can loop or run it through transform (which just loops for you).
the loop is just as good but this is nice and geeky:

string data = "MiXeD"
std::transform(data.begin(), data.end(), data.begin(),
[](unsigned char c){ return std::tolower(c); });

then you can sort it and use that idea.
you could also jack the tolower into a custom comparison function for the sort (and the string compare afterwards). either way.

[noobstudent]

@Furry Guy - I think my brain must have been switching off when I asked, haha! Thanks.

@jonnin - That does look super geeky! :D Thank you. I will play with that once I've finished the current exercise I am on. (Finished the anagram one earlier today and am trying to move through these exercises at a decent pace).

For this next exercise I am currently trying to create a function which replaces 'c' with 'k' - why, in the case of my code, am I getting this output (sorry I didn't translate the names, I figured the program is pretty self-explanatory :) ):

$ ./cani
Introduce una frase: caca
`

#include <iostream>
#include <cstring>
using namespace std;

const int TAM = 100;

typedef char Tfrase[TAM];

void susticionCporK(char caracter, Tfrase normal, Tfrase cani, int i);

int main()
{
    Tfrase normal;
    Tfrase cani;
    int i;
    
    cout << "Introduce una frase: ";
    cin.get(normal, TAM);
    cin.get();
    
    susticionCporK(normal[i], normal, cani, i);
    
    cout << "Versión cani: " << cani;
    
    return 0;
}

void susticionCporK(char caracter, Tfrase normal, Tfrase cani, int i)
{
    int longitud;
    
    i = 0;
    longitud = strlen(normal);
    
    for(i=0; i<longitud; i++)
    {
        if(caracter == 'c')
        {
            caracter = 'k';
        }
    }
    
    cani = normal;
}

[jonnin]

for(i=0; i<longitud; i++)
{
if(caracter == 'c')
{
caracter = 'k';
}
}

you loop who knows how many times, checking the same character over and over.
this seems like it is not exactly what you wanted, as it is identical to

if(caracter == 'c')
{
caracter = 'k';
}

I think you want normal[i] instead of caracter.

int longitud;

i = 0; //!!!! this i and the i in the for loop are NOT THE SAME i!! the i on line 32 is not used and should be discarded. You now have THREE DIFFERENT int i in this code: an incoming parameter, the local one you don't need, and the loop variable one.
longitud = strlen(normal);

for(i=0; i<strlen(normal); i++) //just put the strlen here, unless you know that it is computed every loop and inefficient and you care about the speed.

[seeplus]

Use std::string rather than a c-style array of char. For a char array, cani = normal does not do what you're thinking it does.

#include <iostream>
#include <string>

void susticionCporK(const std::string& normal, std::string& cani);

int main()
{
	std::string normal;
	std::string cani;

	std::cout << "Enter text: ";
	std::getline(std::cin, normal);

	susticionCporK(normal, cani);

	std::cout << "Altered text: " << cani << '\n';
}

void susticionCporK(const std::string& normal, std::string& cani)
{
	cani.clear();

	for (const auto& c : normal)
		cani += c == 'c' ? 'k' : c;
}

If you have to use a c-style char array, then:

#include <iostream>

const size_t maxchar {100};
using carry = char[maxchar];

void susticionCporK(const carry normal, carry cani);

int main()
{
	carry normal {};
	carry cani {};

	std::cout << "Enter text: ";
	std::cin.getline(normal, maxchar);

	susticionCporK(normal, cani);

	std::cout << "Altered text: " << cani << '\n';
}

void susticionCporK(const carry normal, carry cani)
{
	for (; *normal; ++normal, ++cani)
		*cani = *normal == 'c' ? 'k' : *normal;
}