HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Swapping characters pairs in a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
charactersswappingstringpairs

Problem

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?

Solution

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.

Code Snippets

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);
}
return new string(array);
return new String(array);

Context

StackExchange Code Review Q#118218, answer score: 3

Revisions (0)

No revisions yet.