Trying to find isogram in a string c++

user_13838942 0
Viewed 712

I am trying to write a code that has two functions: one that determines whether the string is an isogram or not and another one to print the outcome (true or false) to the console (for the purpose of solving the task).

Some of the things are not working correctly though. And I wonder where I need to improve the code (probably all over...). I would appreciate any advice :)

#include <iostream>
#include<string>
#include<bits/stdc++.h>
#include<iomanip>


bool find_Isogram (std::string str)
{
std::sort(str.begin(), str.end()); //sorted the string for the for loop (e.g. eHllo)

 int length = str.length();

 for (int i = 0; i < length; i++)
    {
    if (str.at(i) == str.at(i+1))
        {
            return false;
            break;
        }
    else
    {
        return true;

    }
}
}

void print_result()
{
     std::string str;
     if (!find_Isogram (str))
     {
      std::cout << "false" << std::endl;
     }

else
{
   std::cout << "true" << std::endl;
}

}

int main()
{

find_Isogram ("gdtub");
print_result();


return 0;

};
````````````````````````````````````````````````````
for-loop c++ string
2 Answers

The condition where you check the consecutive characters for equality is wrong. It will yield true for strings like ABAB. You instead need to use a map with count of each character that has appeared.

Something like:

std::map<char, int> map_of_chars;
for(int i = 0; i < length; i++) {
    map_of_chars[str.at(i)] = map_of_chars[str.at(i)] + 1;
}

If any value in the map is more than 1 return false;

Another implementation would be using the return value of std::unique():

std::sort(str.begin(), str.end());
auto intial_size = str.size();
std::unique(str.begin(), str.end());
if(str.size() == initial_size) {
    /is an isogram
}
else {
    //is not an isogram
}
user_119306020

There are some problems here:

1) You always check an empty string:

print_result will just check an empty string, but it's redundant anyway.

void print_result()
{
     std::string str;         // empty string
     if (!find_Isogram (str)) // finding isogram on empty string
     {
         std::cout << "false" << std::endl;
     }
     ...
}

It can be simplified with std::boolalpha that allows you to print a bool as "true" or "false" (instead of 1 or 0). main would become

int main()
{
    std::cout << std::boolalpha << find_Isogram ("gdtub"); // prints true or false
};

2) Isogram check always ends after first character

Take a look at the condition in find_Isogram. It has a return-statement in the if and else, so you always return after checking the first character. The idea to detect duplicate characters this way is correct (except for the off-by-one-error already mentioned by others). But you want to return true; only after checking all of the characters, e.g. outside the loop:

bool find_Isogram (std::string str)
{
    std::sort(str.begin(), str.end()); //sorted the string for the for loop (e.g. eHllo)

    int length = str.length();

    for (int i = 0; i < length - 1; i++)
    {
        if (str.at(i) == str.at(i+1))
        {
            return false; // whoops duplicate char, stop here
        }
    }    
  
    return true; // no duplicates found, it's an isogram
}
For some further C++-magic, you could simplify it even more with standard library functions :D 
bool find_Isogram (std::string str)
{
    std::sort(str.begin(), str.end());
    return std::unique(str.begin(), str.end()) == str.end();
}
user_51059490
Related