Skip to main content
added 52 characters in body
Source Link
VisualMelon
  • 7.6k
  • 23
  • 46

Here are the results. Linq is as tinstaafl's, StringBuilder is as Joe C's, Char2 is as OP's second method, Char1a and Char1b are variations of what I would have suggested off-hand. On this machine (old i7), under .NET Core 2/.1, in a dedicated benchmark, the OP's code was significantly faster than the Linq and StringBuilder methods. (Results may be very different under .NET Framework)

Personally, I have no issue with your second piece of code. It may not be the most obvious implementation ever, but it doesn't take long to work out, and it's not a method whose job is going to change any time soon, so I'd worry much more about its API than it'sits internals. That said, the API is a problem, as Adriano Repetti has mentioned: the behaviour of this method is going to create problems as soon as you start trying to reverse non-trivial Unicode. Simply, 'reverses a string' is a deficient contract.

Here are the results. Linq is as tinstaafl's, StringBuilder is as Joe C's, Char2 is as OP's second method, Char1a and Char1b are variations of what I would have suggested off-hand. On this machine (old i7), under .NET Core 2/1, in a dedicated benchmark, the OP's code was significantly faster than the Linq and StringBuilder methods.

Personally, I have no issue with your second piece of code. It may not be the most obvious implementation ever, but it doesn't take long to work out, and it's not a method whose job is going to change any time soon, so I'd worry much more about its API than it's internals. That said, the API is a problem, as Adriano Repetti has mentioned: the behaviour of this method is going to create problems as soon as you start trying to reverse non-trivial Unicode. Simply, 'reverses a string' is a deficient contract.

Here are the results. Linq is as tinstaafl's, StringBuilder is as Joe C's, Char2 is as OP's second method, Char1a and Char1b are variations of what I would have suggested off-hand. On this machine (old i7), under .NET Core 2.1, in a dedicated benchmark, the OP's code was significantly faster than the Linq and StringBuilder methods. (Results may be very different under .NET Framework)

Personally, I have no issue with your second piece of code. It may not be the most obvious implementation ever, but it doesn't take long to work out, and it's not a method whose job is going to change any time soon, so I'd worry much more about its API than its internals. That said, the API is a problem, as Adriano Repetti has mentioned: the behaviour of this method is going to create problems as soon as you start trying to reverse non-trivial Unicode. Simply, 'reverses a string' is a deficient contract.

Source Link
VisualMelon
  • 7.6k
  • 23
  • 46

Performance

I didn't believe that the LINQ method could be faster, and I would never trust a profiler to give an accurate result (for numerous reasons), so I ran a benchmark with BenchmarkDotNet, and got the opposite result from tinstaafl. (Code in a gist)

Here are the results. Linq is as tinstaafl's, StringBuilder is as Joe C's, Char2 is as OP's second method, Char1a and Char1b are variations of what I would have suggested off-hand. On this machine (old i7), under .NET Core 2/1, in a dedicated benchmark, the OP's code was significantly faster than the Linq and StringBuilder methods.

                Method |            TestString |         Mean |      Error |    StdDev |
---------------------- |---------------------- |-------------:|-----------:|----------:|
           ReverseLinq |                       |    81.472 ns |  0.1537 ns | 0.1284 ns |
         ReverseChar1a |                       |     7.946 ns |  0.1156 ns | 0.1081 ns |
         ReverseChar1b |                       |     7.518 ns |  0.0177 ns | 0.0157 ns |
          ReverseChar2 |                       |     7.507 ns |  0.0232 ns | 0.0206 ns |
 ReverseStringBuilders |                       |    12.894 ns |  0.1740 ns | 0.1542 ns |
           ReverseLinq |  It's (...)ow it [39] |   671.946 ns |  1.9982 ns | 1.8691 ns |
         ReverseChar1a |  It's (...)ow it [39] |    61.711 ns |  0.0774 ns | 0.0604 ns |
         ReverseChar1b |  It's (...)ow it [39] |    61.952 ns |  0.2241 ns | 0.1986 ns |
          ReverseChar2 |  It's (...)ow it [39] |    48.417 ns |  0.0877 ns | 0.0732 ns |
 ReverseStringBuilders |  It's (...)ow it [39] |   203.733 ns |  0.7540 ns | 0.6684 ns |
           ReverseLinq |               Magpies |   235.176 ns |  0.5324 ns | 0.4446 ns |
         ReverseChar1a |               Magpies |    23.412 ns |  0.0979 ns | 0.0916 ns |
         ReverseChar1b |               Magpies |    24.032 ns |  0.0582 ns | 0.0544 ns |
          ReverseChar2 |               Magpies |    22.401 ns |  0.1193 ns | 0.0996 ns |
 ReverseStringBuilders |               Magpies |    44.056 ns |  0.1313 ns | 0.1097 ns |
           ReverseLinq | ifhia(...) oiha [432] | 4,102.307 ns | 10.4197 ns | 9.2368 ns |
         ReverseChar1a | ifhia(...) oiha [432] |   454.764 ns |  1.0899 ns | 1.0195 ns |
         ReverseChar1b | ifhia(...) oiha [432] |   453.764 ns |  2.3080 ns | 2.0460 ns |
          ReverseChar2 | ifhia(...) oiha [432] |   400.077 ns |  1.0022 ns | 0.7824 ns |
 ReverseStringBuilders | ifhia(...) oiha [432] | 1,630.961 ns |  6.1210 ns | 5.4261 ns |

Note: never used BenchmarkDotNet before... hopefully I've not misused/misunderstood it in any way (please comment if I have), and hopefully it is good at it's job.

Commentary

Performance is not everything. The linq method is the most compact, and the hardest to get wrong, which is very good. However, if performance is important, than you need to profile the method as realistically as possible. The results above may not generalise. However, I'd be very surprised if the StringBuilder and Linq methods out-performed any of the char-array based methods ever, because they just incur a fair amount of overhead (i.e. probably a dynamic array, and probably a second copy in the LINQ case (not to mention the general enumeration overhead)).

Personally, I have no issue with your second piece of code. It may not be the most obvious implementation ever, but it doesn't take long to work out, and it's not a method whose job is going to change any time soon, so I'd worry much more about its API than it's internals. That said, the API is a problem, as Adriano Repetti has mentioned: the behaviour of this method is going to create problems as soon as you start trying to reverse non-trivial Unicode. Simply, 'reverses a string' is a deficient contract.