Reverse Vowels in a String in C#

Question

I am trying to take a string as input and return the string with only the vowels reversed. It is error free but not bug free. It is failing on Test19() and Test20. Running out of ideas how to fix this or why I am having issues on this test. All other test pass. First code block is my code followed by the test I created in the second block.

This is the link to the page that details more information on the test I made. Nunit.org StringAssert (NUnit 2.2.3)

using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Vowels
{
    public class reverseVowels
    {
        public static bool IsVowel(char c)
        {
            char[] vowels = new[] { 'a', 'e', 'i', 'o', 'u', 'A', 'E', 'I', 'O', 'U' };
            return vowels.Any(ch => ch == c);
        }

        public static string reverseVowelsOfString(string s)
        {
            List<char> list = (from char c in s
                               where IsVowel(c)
                               select c).ToList();
            StringBuilder sb = new StringBuilder();

            foreach (char v in s)
            {
                if (IsVowel(v))
                {
                    sb.Append(list.Last());
                    list.Remove(list.Last());
                }
                else
                {
                    sb.Append(v);
                }
            }

            return sb.ToString();
        }
    }
}

using NUnit.Framework;

namespace Vowels
{
    public class VowelsTest
    {
        [Test]
        public void Test1() => StringAssert.Contains("hollo, werld", reverseVowels.reverseVowelsOfString("hello, world"));

        [Test]
        public void Test2() => StringAssert.Contains("cadisegnol", reverseVowels.reverseVowelsOfString("codesignal"));

        [Test]
        public void Test3() => StringAssert.Contains("UOaIye", reverseVowels.reverseVowelsOfString("eIaOyU"));

        [Test]
        public void Test4() => StringAssert.Contains("", reverseVowels.reverseVowelsOfString(""));

        [Test]
        public void Test5() => StringAssert.Contains(" ", reverseVowels.reverseVowelsOfString(" "));

        [Test]
        public void Test6() => StringAssert.Contains(".a", reverseVowels.reverseVowelsOfString(".a"));

        [Test]
        public void Test7() => StringAssert.Contains("ia", reverseVowels.reverseVowelsOfString("ai"));

        [Test]
        public void Test8() => StringAssert.Contains("Aa", reverseVowels.reverseVowelsOfString("aA"));

        [Test]
        public void Test9() => StringAssert.Contains(".,", reverseVowels.reverseVowelsOfString(".,"));

        [Test]
        public void Test10() => StringAssert.Contains("ab", reverseVowels.reverseVowelsOfString("ab"));

        [Test]
        public void Test11() => StringAssert.Contains("0P", reverseVowels.reverseVowelsOfString("0P"));

        [Test]
        public void Test12() => StringAssert.Contains("!!!", reverseVowels.reverseVowelsOfString("!!!"));

        [Test]
        public void Test13() => StringAssert.Contains("a a", reverseVowels.reverseVowelsOfString("a a"));

        [Test]
        public void Test14() => StringAssert.Contains("abb", reverseVowels.reverseVowelsOfString("abb"));

        [Test]
        public void Test15() => StringAssert.Contains("1b1", reverseVowels.reverseVowelsOfString("1b1"));

        [Test]
        public void Test16() => StringAssert.Contains("abba", reverseVowels.reverseVowelsOfString("abba"));

        [Test]
        public void Test17() => StringAssert.Contains("c#dc", reverseVowels.reverseVowelsOfString("c#dc"));

        [Test]
        public void Test18() => StringAssert.Contains("......a.....", reverseVowels.reverseVowelsOfString("......a....."));

        [Test]
        public void Test19() => StringAssert.Contains("raca e car", reverseVowels.reverseVowelsOfString("race a car"));

        [Test]
        public void Test20() => StringAssert.Contains("SorE was I ere I saw eros.", reverseVowels.reverseVowelsOfString("Sore was I ere I saw Eros."));

        [Test]
        public void Test21() => StringAssert.Contains("a man, a plan, a canal -- PanamA", reverseVowels.reverseVowelsOfString("A man, a plan, a canal -- Panama"));
    }
}

  X Test19 [43ms]
  Error Message:
     Expected: String containing "raca e car"
  But was:  "raca a cer"

  Stack Trace:
     at Vowels.VowelsTest.Test19() in C:\Users\scott\Desktop\reverseVowelsOfString\test.cs:line 62

  X Test20 [< 1ms]
  Error Message:
     Expected: String containing "SorE was I ere I saw eros."
  But was:  "Soro wEs a arI I sew eres."

  Stack Trace:
     at Vowels.VowelsTest.Test20() in C:\Users\scott\Desktop\reverseVowelsOfString\test.cs:line 65

Test Run Failed.
Total tests: 21
     Passed: 19
     Failed: 2
 Total time: 1.1547 Seconds

Answer 1

I would make a few changes. First, I would move the creation of the list of vowels into a one-time allocated class member to avoid unnecessary allocations. I'd also make it a HashSet<char> for efficiency reasons:

    private static readonly HashSet<char> _vowels = new HashSet<char>(new[] { 'a', 'e', 'i', 'o', 'u', 'A', 'E', 'I', 'O', 'U' });

Now your IsVowel() method can be simplified to just:

    public static bool IsVowel(char c) => _vowels.Contains(c);

For the method doing all the work, I'd give the variables more meaningful names. vowels instead of list and c instead of v since we know it's a char, but not necessarily a vowel (I'm assuming that's what v stood for).

I'd also put the list of vowels in a Stack<char> so you can easily pop them off as needed.

    public static string reverseVowelsOfString(string s)
    {
        Stack<char> vowels = new Stack<char>(s.Where(IsVowel));
        StringBuilder sb = new StringBuilder();

        foreach (char c in s)
        {
            if (IsVowel(c))
            {
                sb.Append(vowels.Pop());
            }
            else
            {
                sb.Append(c);
            }
        }

        return sb.ToString();
    }

Answer 2

This happens because in test case 19;

list = { 'a', 'e', 'a', 'a' }

And when you try to remove list.Last() which is 'a', the first element is removed.

sb.Append(list.Last()); // appends the last element in the list, which is OK,
list.Remove(list.Last()); // since list.Last() is 'a', removes the first 'a' element in the list.

The solution is as simple as:

sb.Append(list.Last());
list.RemoveAt(list.Count - 1); // Remove the last item in the list.

Hope this helps.