Converting a string into an array and returning the values spelled in reverse

Question

I must create a function that accepts a string of names all separated by a comma.

Example: "Jack,Matthew,John,Sam"

And return the string of names in the same order but spelled in reverse.

Expected output: "kcaJ,wehttaM,nhoJ,maS"

I completed the function with an inner for loop but I would like to know if maybe running a single for loop and inside using a Linq function or something similar, would have been better?

public string Reverse(string s)
{
 var names = s.Split(",");
 string temp = "";
 for (int i = 0; i < names.Length; i++)
 {
 for (int j = names[i].Length - 1; j >= 0; j--)
 {
 temp += names[i].ElementAt(j);
 }
 temp += i == names.Length - 1 ? "" : ",";
 }
 return temp;
}

Anyway I found this problem very simple even though I tried taking my time on it, I found this solution almost immediately so I went with it. I am thinking maybe if I spent more time on it then I could have implemented this with some sort of Linq function to reverse the spelling of the names and removed the inner loop.

Really any suggestions for improving this small function is appreciated.

• user228914: Did you forget to return temp? :)
• stupidQuestions: Nice catch, forgot that.
• user228914: Related : "Easy way to reverse each word in a string"

Answer 1

Good code, few suggestions.

---

Instead of having to check each and every time to make sure you can add a "," or not, add it for all the cases, use String.TrimEnd to remove the extra leading ","

 var names = s.Split(",");
 string temp = "";
 for (int i = 0; i < names.Length; i++)
 {
 for (int j = names[i].Length - 1; j >= 0; j--)
 {
 temp += names[i].ElementAt(j);
 }
 temp += ",";
 }
 return temp.TrimEnd(',');

With that change, we have the ability to foreach loop to further improve the readability of the code.

 string temp = "";
 foreach (var name in s.Split(','))
 {
 for (int j = name.Length - 1; j >= 0; j--)
 {
 temp += name.ElementAt(j);
 }
 temp += ",";
 }
 return temp.TrimEnd(',');

That's good. Although we can improve this further by using a better (faster and cleaner) method to reverse the individual names, and that is to use String.ToCharArray and Array.Reverse

The idea is to convert the string into a character array, use the inbuilt Reverse method to reverse it, add it to temp.

 string temp = "";
 foreach (var name in s.Split(','))
 {
 char[] NameArray = name.ToCharArray();
 Array.Reverse(NameArray);
 temp = temp + new string(NameArray) + ",";
 }
 return temp.TrimEnd(',');

You can also use StringBuilder which is slightly faster

 StringBuilder reversed = new StringBuilder();
 foreach (var word in str.Split(','))
 {
 char[] singlesentence = word.ToCharArray();
 Array.Reverse(singlesentence);
 reversed.Append(singlesentence);
 reversed.Append(",");
 }
 return reversed.ToString().TrimEnd(',');

Nitpick:

• Prefer using a better name like reversed compared to temp, it tells more to the reader.

• Use string.Empty and not ""

---

Benchmarks

Your original code

 static public string Reverse(string s)
 {
 var names = s.Split(",");
 string temp = "";
 for (int i = 0; i < names.Length; i++)
 {
 for (int j = names[i].Length - 1; j >= 0; j--)
 {
 temp += names[i].ElementAt(j);
 }
 temp += i == names.Length - 1 ? "" : ",";
 }
 return temp;
 }

---

New

 static public string Reverse2(string s)
 {
 
 string reversed = string.Empty;
 foreach (var name in s.Split(','))
 {
 char[] NameArray = name.ToCharArray();
 Array.Reverse(NameArray);
 reversed += new string(NameArray) + ",";
 }
 return reversed.TrimEnd(',');
 }

---

New2

 static public string Reverse3(string str)
 {
 StringBuilder reversed = new StringBuilder();
 foreach (var word in str.Split(','))
 {
 char[] singlesentence = word.ToCharArray();
 Array.Reverse(singlesentence);
 reversed.Append(singlesentence);
 reversed.Append(",");
 }
 return reversed.ToString().TrimEnd(',');
 }

---

Note that I have static to use the function without an object reference.

I will use the following method to call the two functions multiple times, and benchmark it.

static void Perform(Func<string, string> Function, string names, int iterations, string type)
{
 var watch = System.Diagnostics.Stopwatch.StartNew();
 for (int i = 0;i < iterations;i++)
 Function(names);
 watch.Stop();
 var elapsed = watch.ElapsedMilliseconds;
 Console.WriteLine($"Time taken for {type}: {elapsed} ms");
}

 static void Main()
 {
 string names = "Jack,Dylan,Bobby,Peter";
 const int iterations = 1000000;
 Perform(Reverse, names, iterations, "Original");
 Perform(Reverse2, names, iterations, "New");
 Perform(Reverse3, names, iterations, "New2");
 
 }

Here are the results

/*
" Release " Configuration
 Iterations | Original | New | New2
-------------+-------------+-----------+--------
 10 000 | 29 ms | 5 ms | 5 ms
-------------+-------------+-----------+--------
 100 000 | 226 ms | 54 ms | 43 ms
-------------+-------------+-----------+--------
 1 000 000 | 1763 ms | 442 ms | 405 ms
*/

Try it here

• BCdotWEB: But why use a StringBuilder instead of simply filling a collection and then applying String.Join afterwards (this also does away with the ugly .TrimEnd(','))? Especially considering this: stackoverflow.com/questions/585860/… (Quite frankly, even if the performance of a StringBuilder was better, I'd still opt for the String.Join approach since it is more readable IMHO.)
• user228914: @BCdotWEB Yeah that's true, the thing is String Builder is faster. If compactness and readability weigh-out performance in your project, you're welcome to use it :)
• user228914: @BCdotWEB You mean using string.Join(",", str.Split(',').Select(x => new String(x.Reverse().ToArray()))); right?
• user228914: @BCdotWEB I've benchmarked the performance here. To me, the difference is too significant.

Answer 2

You asked about Linq, so I decided to add a way to do it purely with Linq. This performs a bit worse than the StringBuilder examples, but is still 3x faster than the original:

public static string ReverseWithLinq(string str)
{
 return string.Join(
 ',', 
 str
 .Split(',')
 .Select(w => new string(w.Reverse().ToArray())));
}

P.S. I would typically prefer using framework capabilities as much as possible, and only optimize further when necessary.