Remove all vowel from file C++

Question

i am just start to learn C++ and would like to know if there is a better and efficient way to solve this problem or some recommendations.

Problem: Remove all vowels from file and create new one without vowels.

Here is my solution.

#include <iostream>
#include <fstream>

bool is_vowel(char &ch) {
    ch = tolower(ch);//clion showing warning here(value of type 'int' may not fit into type 'char') ,dont know why , i havent int here.
    return (ch == 'a' || ch == 'o' || ch == 'u' || ch == 'i' || ch == 'e');
}

int main() {
    ifstream file{"data.txt"};
    ofstream outfile{"new.txt"};
    string word;
    getline(file, word);
    for (char &ch : word) {
        if (!is_vowel(ch))
            outfile << ch;
    }
}

Answer 1

bool is_vowel(char &ch) {
    ch = tolower(ch);//clion showing warning here(value of type 'int' may not fit into type 'char') ,dont know why , i havent int here.
    return (ch == 'a' || ch == 'o' || ch == 'u' || ch == 'i' || ch == 'e');
}

The warning, is because tolower returns an integer. You could write this:

ch = static_cast<char>( ::tolower(ch) );

The condition is fine, but I would have written it like this:

static const auto vowels = { 'a', 'e', 'i', 'o', 'u' };
using namespace std;
return end(vowels) == find(begin(vowels), end(vowels), ch);

This separates the act of checking for vowels from defining what vowels are (which is good for maintenance; for example, you could decide you also need ă as a vowel, later).

The function bool is_vowel(char &ch) alters the value of the argument, but you do not use the resulting value, in the calling code.

The function could be written like this:

bool is_vowel(char ch) // ch not passed by reference
{
    static const auto vowels = { 'a', 'e', 'i', 'o', 'u' };
    using namespace std;
    return end(vowels) == find(begin(vowels), end(vowels),
        static_const<char>( ::tolower(ch) );
}

The client code doesn't get all vowels from file, just from the first line.

Correct code should keep reading likes from the input file until it cannot get any, or read character by character, instead of line by line:

Line by line:

while(getline(file, word)) // notice 'while' here
    for (char &ch : word)
        if (!is_vowel(ch))
             outfile << ch;

Character by character:

auto c = char{ 0 };
std::noskipws(file); // do not skip spaces when looking for the next char.
while(file >> c)
    if(is_vowel(c))
        outfile << c;

Answer 2

For the moment, let's ignore is_vowel itself, and just look at the higher level: what we're doing is copying data from some source to some destination, while removing any items that meet some specified criteria.

As it happens, the standard library already has an algorithm to do exactly that: std::remove_copy_if (I know: crappy name; not clear what it really does--but that really is what it does).

So, I will posit that at the top level, this is how we should be carrying out the overall task:

remove_copy_if(some_input, some_output, your_criteria);

In the case of files, we can use istream_iterators for the input and an ostream_iterator for the output, making this something like:

std::ifstream file{"data.txt"};
std::ofstream outfile{"new.txt"};

file >> std::noskipws;

std::istream_iterator<char> in{file}, end;
std::ostream_iterator<char> out{outfile};

std::remove_copy_if(in, end, out, is_vowel);

Looking at is_vowel itself: it can be defined a number of different ways, depending on the degree to which you care about speed vs. space usage. If you care primarily about space, storing the vowels in a string and searching that string (e.g., as suggested by @utnapistim) is a perfectly fine method. Under the circumstances (copying from one file to another) the I/O will almost certainly be (quite a lot) slower than the processing, so this is probably the most sensible approach.

For another possibility, however, let's assume our source and destination were just strings in memory instead, so we wanted to optimize for speed instead. In this case, we might consider a somewhat different approach. One possibility would be an array (or vector) of Booleans, one for each possible char value, specifying whether that character is a vowel. We probably want to wrap this up in a neat little class so we don't need to deal with the details.

class is_vowel {
    bool table[std::numeric_limits<unsigned char>::max()];
    static const char vowels = "aeiouAEIOU";
public:
    // Here we set up our table. We start by initializing everything to `false`
    // then we set the entries that really do represent vowels to true.
    is_vowel() : table(false) {
        for (auto ch : vowels)
            table[ch] = true;
    }

    bool operator()(unsigned char ch) { return table[ch]; }
};

Unfortunately, it's not always easy to decide which sort of approach will be the best for a given circumstance, but such is life sometimes.

There are also a few intermediate points that probably don't make sense for this specific case, but can for similar cases. For example, here we're dealing with such a small number of items (around 10) that searching the string isn't particularly slow. If we had a list of, say, a few hundred items we wanted to remove, searching the whole list every time could get pretty slow--for such cases, we might consider sorting the list of items to remove, and using a binary search.

I'm starting to get into pretty hypothetical areas though, so perhaps I should just stop for now... :-)

Answer 3

You can remove ch == 'a' || ... if you introduce the following function:

bool member(char ch,char *str){
    size_t len = strlen(str);
    for(size_t i=0;i!=len;i++)
        if(ch==str[i])return true;
    return false;
}

is_vowel becomes:

bool is_vowel(char ch){
    return member(ch,"aeiouAEIOU");
}

Simplyfiyng the definition of is_vowel while you gain a fuction to check if a character is part of a string.