1

Kindly assist to make this code insert ob after every vowel in a string entered by the user in C++. The code is adding a double ob before only the first vowel. Thanks.

#include <iostream>
#include <string>
#include <algorithm>

int main()
{
    std::string s;
    std::cin >> s;
    std::string o ="ob";
    for (char& v: s)
    {
        if (v == 'a' || v=='e' || v == 'i' || v == 'o' || v == 'u')
            s = s.insert(s.find(v),o);
    }
    std::cout << s << std::endl;
}
Richard
  • 8,920
  • 2
  • 18
  • 24
  • 1
    One problem is that you use `s.find(v)` : this will return the same location for every vowel of the string. Instead, you should iterate using iterators, and then you don't need `s.find(v)`because you have the iterator. Does it make sense? – Olivier Sohn Jul 02 '18 at 23:59
  • 1
    Note that using iterators will also fix another issue in your code, which is that you modify the string while iterating on it using a range-based for loop (it is undefined behavouir, read this : https://stackoverflow.com/questions/17076672/add-elements-to-a-vector-during-range-based-loop-c11) – Olivier Sohn Jul 03 '18 at 00:01
  • After inserting "ob", you'll need to advance the starting search position by the length of "ob". – Thomas Matthews Jul 03 '18 at 00:02
  • 2
    There is no need to store the input string. Just use cin.get to get the next character and output it as is or replace it. – zdf Jul 03 '18 at 00:03
  • 2
    You should put the vowels into a string and search the vowel string for the character; this eliminates the long `if` statement. – Thomas Matthews Jul 03 '18 at 00:04
  • There are multiple bugs in the shown code. Other comments have pointed out most of them. But the worst one is that the first call to `insert()` invalidates the range iteration, which results in undefined behavior. You are fortunate that your program doesn't crash and burn. – Sam Varshavchik Jul 03 '18 at 00:35

3 Answers3

1

I'm not going to mention the bugs mentioned in the comments.

I would avoid trying to manipulate the same string being searched.

Particularly with a range-based for loop which will use iterators. Iterators may be invalidated by altering the underlying collection, often coinciding with dynamic memory (re)allocation. I don't know how string iterators work off the top of my head, but I would guess that as you grow the string you're invalidating your iterators.

I would create a second string, scan the first string to determine how many vowels there are, and reserve that much space immediately. This requires iterating over the first string twice, but you should only invoke heap allocation at most once.

Then simply iterate over the first string and populate the second.

#include <iostream>
#include <string>
#include <algorithm>

bool is_vowel(char c)
{
  return c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u';
}

int main()
{
  const std::string vowel_postfix = "ob";
  std::string in, out;
  std::cin >> in;
  auto vowel_count = std::count_if(in.begin(), in.end(), is_vowel);
  out.reserve(in.length() + vowel_count * 2);
  for (char c : in) {
    out.push_back(c);
    if (is_vowel(c))
      out.insert(out.length(), vowel_postfix);
  }
  std::cout << out << std::endl;
}
Rain
  • 321
  • 1
  • 8
  • Now when I use 'push_front' it returns an error, is it because the reserved space is after the vowel. Because I've tried in latest c++ 11 compiler to rule out compiler problems. – Mathews Jumba Jul 03 '18 at 20:54
  • No, std::vector::reserve does not prevent reallocation. Your error is hopefully because std::vector does not define a push_front member function. [cppreference.com](https://en.cppreference.com/w/cpp/container/vector) is a great resource for STL containers, and everything else C++. – Rain Jul 03 '18 at 22:21
0

I would do:

bool is_vowel(char cc)
{
    const char c = tolower(cc);
    return
        c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u' || c == ä || c == ö;
}

... and all other possible vowels with accent marks. Then:

int main()
{
    std::string s;
    std::cin >> s;

    std::string ss;

    for (const char c: s)
    {
        ss += c;

        if (is_vowel(c))
            ss += "ob";    
    }

    std::cout << ss << std::endl
}
0

Try something more like this instead. It uses indexes instead of iterators, and lets the std::string handle the searching and comparisons for you instead of you doing them manually:

#include <iostream>
#include <string>

int main()
{
    const std::string vowels = "aAeEiIoOuU";
    const std::string o = "ob";

    std::string s;
    std::cin >> s;

    auto i = s.find_first_of(vowels);
    while (i != std::string::npos)
    {
        s.insert(++i, o);
        i = s.find_first_of(vowels, i + o.size());
    }

    std::cout << s << std::endl;
}

Live Demo

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770