Thread: Reverse a string but not words (Rate my solution)

You can just use string.split to break it up, then with that array just reverse populate.

Code:

private string ReverseString(string input)
{
  List<string> list1 = new List<string>(input.Split(' '));
  List<string> list2 = new List<string>();
  for(int i = list1.count-1; i>=0; i++)
  {
    list2.add(list1[i]+" ");
  }
  string result = "";
  foreach(string s in list2)
  {
    result+=s;
  }
  return result.Trim();
}

Forgive any typos, just bashing out the code in a rush.

Originally Posted by DanFraser

You can just use string.split to break it up, then with that array just reverse populate.

Code:

private string ReverseString(string input)
{
  List<string> list1 = new List<string>(input.Split(' '));
  List<string> list2 = new List<string>();
  for(int i = list1.count-1; i>=0; i++)
  {
    list2.add(list1[i]+" ");
  }
  string result = "";
  foreach(string s in list2)
  {
    result+=s;
  }
  return result.Trim();
}

Forgive any typos, just bashing out the code in a rush.

Yeah but I wanted to do it without the built in c# methods :P to learn it from scratch

Originally Posted by DanFraser

You can just use string.split to break it up, then with that array just reverse populate.

You could make it even simpler:

Code:

string[] words = input.Split(' ');
Array.Reverse(words);
string result = string.Join(" ", words);

Anyhow, to the original poster, it's fine to do things as exercises but remember not to get too used to reinventing the wheel.

Comments on the code:

1. SPACE is an unnecessary use of a constant. Firstly, the literal representation of a space would never possibly change. Second, giving it the name "space" tells nothing extra about the context. For example, "const int MAX_FILES = 32;" might make sense, but "const int THIRTY_TWO = 32;" never does.

2. It's very inefficient to concatenate single characters onto a string. C# strings are immutable - every time you concatenate a character, it makes a new string object, copies all of the characters from the old string and tacks on one new character. StringBuilder is far better for that - or making a char[] and doing it all yourself. As it is, your code is O(N^2) in complexity; the problem can be solved in O(N).

3. I do find it a little odd that you iterate backwards rather than forwards. It certainly will still work, it just is a bit unusual. I would probably have opted to split the input into an array of strings and joined them at the end in reverse order. That may be just personal preference though.

You can even do it in one line by virtue of a stack, which naturally iterates over its items in LIFO order!

Code:

Console.WriteLine(String.Join(" ", new Stack<string>("how are you".Split(' '))));

Code:

you are how

Originally Posted by itsme86

You can even do it in one line by virtue of a stack, which naturally iterates over its items in LIFO order!

Code:

Console.WriteLine(String.Join(" ", new Stack<string>("how are you".Split(' '))));

Code:

you are how

That is... beautiful...

Originally Posted by Cat

3. I do find it a little odd that you iterate backwards rather than forwards. It certainly will still work, it just is a bit unusual. I would probably have opted to split the input into an array of strings and joined them at the end in reverse order. That may be just personal preference though.

There's nothing odd about iteration in reverse order when IF, it makes sense to do so. Why repopulate the entire array so you can iterate it from index 0 to the last upperbound index? That's just an extra, and unnecessary step, unless you need the array in reverse order for other purposes. It can actually optimized to iterate in reverse in this case.

If you have a large enough string as you said, it may be best to iterate in reverse with a StringBuilder so that the overhead catches up and provides faster running time in the long run. For shorter strings, I would just recommend Array.Reverse() here.

Originally Posted by itsme86

You can even do it in one line by virtue of a stack, which naturally iterates over its items in LIFO order!

Code:

Console.WriteLine(String.Join(" ", new Stack<string>("how are you".Split(' '))));

Code:

you are how

By my impression or preference, I would find this more odd, that you would rather create an array, and instantiate a new instance of the Stack class, just so you can have a one-liner piece of code. Less code is not always better. LINQ is a perfectly good example for supporting evidence of my statement on this one.

To mention: the Stack<T> generic class uses the same princpile behind what OP tried to do with the reverse iteration anyways with internal arrays. It takes the array size and (-1) to get the upperbound for the last element, and depending on whether you are just looking at this value or taking it off the stack, that Length property changes, and is used as a reference the next time the last element is called to be shown or pop'd off. So it may not be so strange after all.

Essentially the Stack class usage in your example is just acting as a wrapper for what he initially tried to attempt to do, and for what you called "odd". Only instantiation of the class for everything else that you probably don't need here is a burden.

~cstryx

Originally Posted by cstryx

There's nothing odd about iteration in reverse order when IF, it makes sense to do so. Why repopulate the entire array so you can iterate it from index 0 to the last upperbound index? That's just an extra, and unnecessary step, unless you need the array in reverse order for other purposes. It can actually optimized to iterate in reverse in this case.

Look at the original algorithm. Every word gets inverted twice (tmpCpy becomes a backwards word, which is then inverted again when copied to reversedString). That will never be an optimal solution - string copying a word at a time is more efficient than inverting every word.

By my impression or preference, I would find this more odd, that you would rather create an array, and instantiate a new instance of the Stack class, just so you can have a one-liner piece of code. Less code is not always better. LINQ is a perfectly good example for supporting evidence of my statement on this one.

Well, that wasn't my code, but I would argue that less code, if well-written, is usually better. Unless you've got highly specialized needs or are running this on huge data sets, the actual performance differences of various algorithms is likely to be effectively irrelevant. When performance isn't a constraint, the optimal solution becomes that which is most readable, maintainable, and reliable - what code would be most easily understood and least likely to contain bugs?

Every bug costs hundreds of dollars, and more importantly it pulls people's time away from developing new features. A human can tolerate a response time of up to 100 milliseconds and still feel that the system is reacting "instantly" to their input, and up to a second delay is tolerable without losing continuity of thought. When your data processing is happening orders of magnitude faster than that, the performance consideration is insignificant compared to the maintenance consideration. I just tested my simple code (split, array.reverse, string.join) on the first fifty chapters of the Bible (the book of Genesis). It took four milliseconds on my five year old PC. Extrapolating to the full Bible, without any special concerns for performance I could reverse that text "instantly" (from an end user's point of view).

Yes, there certainly can be applications where performance matters (I've worked on a number of such applications), but ninety-nine times out of a hundred, you gain more by writing code for readability and maintainability than performance.

Originally Posted by Cat

Look at the original algorithm. Every word gets inverted twice (tmpCpy becomes a backwards word, which is then inverted again when copied to reversedString). That will never be an optimal solution - string copying a word at a time is more efficient than inverting every word.

I was posting not upon his method, but your statement, here: "I do find it a little odd that you iterate backwards rather than forwards."

There's nothing wrong with reverse iteration. You could split the words, and traverse the array in descending order to retrieve the elements from last to first sequentially, and it would be fine.

Originally Posted by Cat

Well, that wasn't my code, but I would argue that less code, if well-written, is usually better. Unless you've got highly specialized needs or are running this on huge data sets, the actual performance differences of various algorithms is likely to be effectively irrelevant. When performance isn't a constraint, the optimal solution becomes that which is most readable, maintainable, and reliable - what code would be most easily understood and least likely to contain bugs?

Every bug costs hundreds of dollars, and more importantly it pulls people's time away from developing new features. A human can tolerate a response time of up to 100 milliseconds and still feel that the system is reacting "instantly" to their input, and up to a second delay is tolerable without losing continuity of thought. When your data processing is happening orders of magnitude faster than that, the performance consideration is insignificant compared to the maintenance consideration. I just tested my simple code (split, array.reverse, string.join) on the first fifty chapters of the Bible (the book of Genesis). It took four milliseconds on my five year old PC. Extrapolating to the full Bible, without any special concerns for performance I could reverse that text "instantly" (from an end user's point of view).

Yes, there certainly can be applications where performance matters (I've worked on a number of such applications), but ninety-nine times out of a hundred, you gain more by writing code for readability and maintainability than performance.

Every bug costs hundreds of dollars, and more importantly it pulls people's time away from developing new features.

I've seen consequently where too much time is put into new features which can do just as much damage to the overall result as well; all other considerations thrown out the window.

You are writing as if performance is everything to do about speed however, which is where I've seen people get into trouble. Performance also includes memory usage as well as quite a few other things, so where something might be more readable, sometimes it has it's downsides in other aspects of performance concepts, which should not be neglected. So the way I think when I write a program, is with balanced considerations in all aspects that matter depending on what task I'm trying to achieve through code.

Readability is important, but good code is it's own documentation in my opinion. (Not in regards to a million lines of comments either, that is also a coding nightmare, but rather programming standards, and structure, with the simplest things like naming conventions among other things, do the job on their own if you're looking at good quality code.)

~Ace

I've done a test.

For loop:

Code:

private static string ReverseString(string input)
{
	string[] parts = input.Split(' ');
	int j = parts.Length - 1;
	for (int i = 0; i < j; i++, j--)
	{
		string tmp = parts[i];
		parts[i] = parts[j];
		parts[j] = tmp;
	}
	return string.Join(" ", parts);
}

vs.

Array.Reverse():

Code:

string[] parts = "one two three four five six".Split(' ');
Array.Reverse(parts);
string.Join(" ", parts);

When I did this test, the Array.Reverse() code was put directly within my benchmark method, and the ReverseString() function was called from the benchmarking method. The results:

Code:

(Benchmark Test... 10 averages of 1000000 runs)

# For loop method
> [Average 1/10] Complete - 3.180357 ticks
> [Average 2/10] Complete - 3.263104 ticks
> [Average 3/10] Complete - 3.305357 ticks
> [Average 4/10] Complete - 3.312106 ticks
> [Average 5/10] Complete - 3.349507 ticks
> [Average 6/10] Complete - 3.265488 ticks
> [Average 7/10] Complete - 3.300283 ticks
> [Average 8/10] Complete - 3.363832 ticks
> [Average 9/10] Complete - 3.29727 ticks
> [Average 10/10] Complete - 3.281331 ticks
# Result: 3.2918635 ticks


(Benchmark Test... 10 averages of 1000000 runs)

# Array.Reverse() Method
> [Average 1/10] Complete - 3.744852 ticks
> [Average 2/10] Complete - 3.829094 ticks
> [Average 3/10] Complete - 3.846189 ticks
> [Average 4/10] Complete - 3.828799 ticks
> [Average 5/10] Complete - 3.824417 ticks
> [Average 6/10] Complete - 3.801133 ticks
> [Average 7/10] Complete - 3.820336 ticks
> [Average 8/10] Complete - 3.813253 ticks
> [Average 9/10] Complete - 3.794533 ticks
> [Average 10/10] Complete - 3.818379 ticks
# Result: 3.8120985 ticks

I added the string.Join() just for making things consistent, I didn't care about using it's value, only calling the method to make things equal.

Understand that Array.Reverse() in the background is lots more complex than the function I wrote namely, ReverseString, taken the name from OP. However, we don't care about that, because by the method name itself, it is clear enough for what the method is supposed to do.

Perhaps even ReverseWords, would be more appropriate for this method name as well.