Swapping characters pairs in a string

Question

I have to swap the characters in a string before sending said string to a device to show the information sent through a LCD, the method I developed is as follows:

private string StringSwap(string stringToSwap)
{
    if ((stringToSwap.Length % 2).Equals(1))
    {
        stringToSwap += " ";
    }

    char[] array = stringToSwap.ToCharArray();

    for (int i = 0; i < array.Length; i += 2)
    {
        char temp = array[i];
        array[i] = array[i + 1];
        array[i + 1] = temp;
    }
    return new string(array);
}

I want to know if there are any implementations that do this already (I feel like I am reinventing the wheel here) or, in case there is not, know how to optimize and make this code more resilient, while also making it more readable.

Also, should I make this an extension for the string class instead of a method?

Answer 1

Here it as an extension. It helps to properly name all the steps:

static class StringSwap
{
    public static string Swap(this string text) =>
        new string(text
            ?.PadToEvenLength()
            ?.SwapOddEvenChars()
            ?.ToArray());

    static string PadToEvenLength(this string text) =>
        text.Length.IsOdd() ? text + ' ' : text;

    static IEnumerable<char> SwapOddEvenChars(this string text) =>
        text.Select((c, i) => i.IsOdd() ? text[i - 1] : text[i + 1]);

    static bool IsOdd(this int number) => 
        number % 2 == 1;        
}

Answer 2

I haven't run the code, but are you sure it does as you expect it to for odd length strings? By padding the last character with a space, you get some... interesting results.

For example, inputting "foo" will return "of o". That doesn't seem quite right to me. I would expect it to return "ofo". I would do something like this.

private string StringSwap(string stringToSwap)
{
    char[] array = stringToSwap.ToCharArray();

    // even size strings iterate the whole array
    // odd size strings stop one short 
    int offset = (stringToSwap.Length % 2)
    for (int i = 0; i < array.Length - offset; i += 2)
    {
        char temp = array[i];
        array[i] = array[i + 1];
        array[i + 1] = temp;
    }
    return new String(array);
}

Essentially, when the mod of 2 equals zero, the code behaves exactly like yours does now, but when the mod of 2 equals one, the loop exits one iteration early, avoiding the IndexOutOfRangeException that prompted you to hack in that white space.

Also note that I changed

    return new string(array);

To

     return new String(array);

It's terribly picky and pedantic, but I find it poor form to use the string alias when you're using it as a class. Seeing a ctor called on a camelCased identifier just looks weird to me. Same goes for things like string.Empty. But, like I said, that's just me being nit picky and pedantic. It's not actually a problem.

Answer 3

Finally, an excuse to use the Zip extension method:

private static string StringSwap2(string stringToSwap)
{
    if ((stringToSwap.Length % 2).Equals(1))
    {
        stringToSwap += " ";
    }

    var evens = stringToSwap.Where((w, i) => i % 2 == 0);
    var odds = stringToSwap.Where((w, i) => i % 2 == 1);

    var zipped = evens.Zip(odds, (left, right) => string.Concat(right, left));

    return string.Concat(zipped);
}

Answer 4

As a slightly different approach:

public class StringHelper
{
    private static void SwapChars(ref char a, ref char b)
    {
        char c = a;
        a = b;
        b = c;
    }

    public static string StringSwap(string stringToSwap)
    {
        if (stringToSwap == null)
        {
            throw new ArgumentNullException("stringToSwap");
        }

        char[] array = stringToSwap.ToCharArray();

        for (int i = 0; i < array.Length - 1; i += 2)
        {
            StringHelper.SwapChars(ref array[i], ref array[i + 1]);
        }

        return new string(array);
    }
}

The other answers are also correct, but I wanted to present a more traditional alternative.

Also I wanted to intoduce handling null as an exception. Trying to swap a null should generally be treated as an error condition. You may decide that for the sake of simplicity you want to convert a null argument to an empty string or make it return null, that's up to you, but whatever you do you should explicitly handle it as an edge-case.

First note that I have changed i < array.Length to i < array.Length - 1, which has the side effect of making the loop stop short of the final char in a string with an odd length. This behaviour may not be obvious at first so you may prefer to comment or use a variation of one of the other answers to make the behaviour more explicit, but this solution is somewhat more concise than explicit handling of the case where the length is odd.

I chose to make charSwap its own function as a means of separating concerns and making the charSwap function available to future helper functions that may need the functionality, particularly if you are planning to create other string swapping functions with variations on the behaviour of this one.