1
\$\begingroup\$

I have the following two approaches for writing a custom string reverse algorithm and I was hoping someone could tell me which is the most efficient / preferable option and explain why. I am fairly new to Java and this would be educational for me. FYI I know this can be done using Stringbuilder but I wanted to write my own solution.

Solution 1: uses two arrays, copying the contents of one into another but in reverse order

//include needed util
import java.util.Scanner;

 /**
 * This class will reverse a string
 * @author Richard
 *
 */

public class ReverseString {
/**
 * This method will take in a string, convert that to two 
 * char arrays, then swap the contents of one for the contents
 * of the other except moving in opposite directions and so
 * will reverse the order of the output string  
 * @param args
 */

public static void main(String[] args) {

    //declare variables
    Scanner input = new Scanner(System.in);
    String inputString = "";
    int length = 0, indexOfArrayToChange = 0, indexOfReferenceArray = 0;

    //get input string
    System.out.print("Enter the string you want to reverse: ");
    inputString = input.nextLine();

    //find length of string, it needs to be reduced by one to prevent an error
    //as index start at 0.
    length = inputString.length()-1;
    //set 'indexOfReferenceArray' equal to string.length-1 for use later in loop
    indexOfReferenceArray = length;

    //convert to arrays
    char[] arrayToChange = inputString.toCharArray();
    char[] referenceArray = inputString.toCharArray();

    //loop through and reverse order as long as we are still within bounds of original string length
    while (indexOfArrayToChange<=length) {
        //change the first letter of one array to be the same as the last letter of reference array
        arrayToChange[indexOfArrayToChange] = referenceArray[indexOfReferenceArray];
        //increment/decrement to move through arrays
        indexOfArrayToChange++;
        indexOfReferenceArray--;

    }
    //convert back to string
    String newString = new String(arrayToChange);

    //output result
    System.out.println(newString);

    //close resources
    input.close();
}
}

Solution 2: uses one array and swaps from back to front meeting in the middle. However the code within the loop has to be longer to do the swapping.

//include needed util
import java.util.Scanner;

/**
* This class will reverse an inputed string
* @author Richard
*
*/
public class ReverseString {

/**
 * This method will take in a string, convert that to a 
 * char array, then swap the contents of the array from back to front
 * meeting in the middle.  
 * @param args
 */
public static void main(String[] args) {
    //declare variables
    Scanner input = new Scanner(System.in);
    String inputString = "";
    int length = 0, indexStart = 0, indexEnd = 0;

    //get input string
    System.out.print("Enter the string you want to reverse: ");
    inputString = input.nextLine();

    //find length of string, it needs to be reduced by one to prevent an error
    //as index start at 0.
    length = inputString.length()-1;
    //set 'indexOfReferenceArray' equal to string.length-1 for use later in loop
    indexEnd = length;

    //convert to arrays
    char[] array = inputString.toCharArray();

    //loop through and reverse order until two vars meet in middle of array
    while (indexStart<=(length/2)) {
        //swap array elements
        char temp = array[indexStart];
        array[indexStart] = array[indexEnd];
        array[indexEnd] = temp;
        //increment/decrement to move through arrays
        indexStart++;
        indexEnd--;
    }

    //convert back to string
    String newString = new String(array);

    //output result
    System.out.println(newString);

    //close resources
    input.close();

}

}
\$\endgroup\$
0
\$\begingroup\$

Solution 2 is (probably, that depends on many factors) the more efficient one, as its (slightly more complex) loop body is executed only length/2 times, and it doesn't create two character arrays from the input string.

But besides answering your question, I'd like to comment on your coding style - we're on codereview, anyway.

First, I'd always separate input/output and computation, so you can re-use the computation as a "library function" with different input/output or as part of a bigger computation.

Then, you often declare a variable before you want to use it (and even assign an unnecessary initial value to it). It's accepted style to declare a variable exactly when you need it first, and assign it the value you want.

You "adjusted" the length value by -1 because you need that value for the endIndex. That's misleading, as now you have a variable named length, but it doesn't contain the length value. This adjustment should be done in the endIndex variable, not the length.

It's good that you are using Javadoc comments. But you should document pieces of code from an "outside" view: what the method does, not how it's doing its job. See my re-worded javadoc for the main() method.

As a matter of personal style, I'd replace your while loop with a for loop that uses two variables: in every iteration, it increases indexStart and decreases indexEnd.

See my modified version of your solution 2:

package test;

import java.util.Scanner;

public class ReverseString {

    /**
     * This method will read a String from standard input 
     * and print the character-reversed string to standard output.  
     * @param args
     */
    public static void main(String[] args) {
        //declare variables
        Scanner input = new Scanner(System.in);
        // Don't introduce variables before you need them
        // String inputString = "";
        // int length = 0, indexStart = 0, indexEnd = 0;

        //get input string
        System.out.print("Enter the string you want to reverse: ");
        String inputString = input.nextLine();

        String newString = reverseString(inputString);

        //output result
        System.out.println(newString);

        //close resources
        input.close();
    }

    /**
     * Returns the character-reversed String of the inputString
     * @param inputString the input String
     * @return the input string in reversed order of the characters
     */
    public static String reverseString(String inputString) {
        //find length of string, it needs to be reduced by one to prevent an error
        //as index start at 0.
        // NO! It's not the length that's wrong, it's the indexEnd
        int length = inputString.length();
        //set 'indexOfReferenceArray' equal to string.length-1 for use later in loop
        // int indexEnd = length;

        //convert to arrays
        char[] array = inputString.toCharArray();

        //loop through and reverse order until two vars meet in middle of array
        // while (indexStart<=(length/2)) {
        // I replaced that with a for loop using two variables
        for (int indexStart=0, indexEnd=length-1; indexStart<indexEnd; indexStart++,indexEnd--) {
            //swap array elements
            char temp = array[indexStart];
            array[indexStart] = array[indexEnd];
            array[indexEnd] = temp;
            //increment/decrement to move through arrays
            // That's done by the for loop, now.
            // indexStart++;
            // indexEnd--;
        }

        //convert back to string
        String newString = new String(array);
        return newString;
    }

}

But I'd write still another version of the program:

package test;

import java.util.Scanner;

public class ReverseString2 {

    /**
     * This method will read a String from standard input 
     * and print the character-reversed string to standard output.  
     * @param args
     */
    public static void main(String[] args) {
        //declare variables
        Scanner input = new Scanner(System.in);
        // Don't introduce variables before you need them
        // String inputString = "";
        // int length = 0, indexStart = 0, indexEnd = 0;

        //get input string
        System.out.print("Enter the string you want to reverse: ");
        String inputString = input.nextLine();

        String newString = reverseString(inputString);

        //output result
        System.out.println(newString);

        //close resources
        input.close();
    }

    /**
     * Returns the character-reversed String of the inputString
     * @param inputString the input String
     * @return the input string in reversed order of the characters
     */
    public static String reverseString(String inputString) {
        int length = inputString.length();
        char[] array = new char[length];

        for (int iSource=0,iTarget=length-1; iTarget>=0; iSource++,iTarget--) {
            array[iTarget] = inputString.charAt(iSource);
        }

        String newString = new String(array);
        return newString;
    }

}
\$\endgroup\$
  • \$\begingroup\$ Or even don't open-code iteration: use a foreach-loop or a util.stream.Stream. \$\endgroup\$ – greybeard Oct 21 '17 at 19:55
0
\$\begingroup\$

In both solutions you're doing the same amount of work. You take each character from the input String, and insert that into the target char array on the required index.

The only difference is that in your second loop, you do twice the work in half the iteration steps.

In such a case it's always better to prioritise readability. Think about someone else reading your code, or you yourself having to read it again in a year or three. Then it should still be obvious at first sight what it does.

For that reason I would prefer your first solution. But I think we can do a little better.

First start by creating a method that does the reversing of a String. That way, you separate the logic of "string reversing" with user input-output. Your main will then look like this:

public static void main(String[] args) {
    Scanner input = new Scanner(System.in);

    System.out.print("Enter the string you want to reverse: ");
    String inputString = input.nextLine();

    String reversedString = reverseString(inputString);

    System.out.println(reversedString);
}

Notice how I also removed all of the comments? The code itself should be clear enough to say what it does without extra comments. Comments are for explaining why you do something. Take this comment for example:

//find length of string, it needs to be reduced by one to prevent an error
//as index start at 0.
length = inputString.length()-1;

It explains why you have to do -1 here. Although every java programmer will probably know this already so it's not really required either.


As for the actual method that will reverse the String. This can also be a bit improved. What you want to do is loop over each character in the input. And add that in reverse order to the result.

There's a nice way to write "for each character in the input" in java. It's called the for-each loop and looks like this:

for(char c : input.toCharArray()){

So the only thing we need to do then is put that character in our result.

public static String reverseString(String input){
    char[] result = new char[input.length()];

    int currentIndex = result.length-1;
    for(char c : input.toCharArray()){
        result[currentIndex--] = c;
    }
    return new String(result);
}

Let me add here how I would read this piece of code when I would revisit it after a couple of years to see what it does. The comments are not in the actual code ofcourse, just to explain it here:

public static String reverseString(String input){
    //we create a char array to store our result
    char[] result = new char[input.length()];

    //start at the last index
    int currentIndex = result.length-1;
    //for each character in the input string
    for(char c : input.toCharArray()){
        //add it at the current index (which started at the end) 
        //and decrement index for the next step
        result[currentIndex--] = c;
    }
    //return the correct type
    return new String(result);
}
\$\endgroup\$
  • \$\begingroup\$ reverseString() lacks a doc comment. (Especially being public!) \$\endgroup\$ – greybeard Oct 24 '17 at 8:24
  • \$\begingroup\$ @greybeard What kind of javadoc do you expect on that method? Where I work, we don't put javadoc on obvious methods. Do you really need more info here? \$\endgroup\$ – Imus Oct 25 '17 at 7:13
  • \$\begingroup\$ Habitually documenting code is not (just) about local necessity: it is about supporting the maintenance programmer, library/jar user, or a decades older self sticking to conventions supported by, e.g., IDEs. \$\endgroup\$ – greybeard Oct 25 '17 at 8:10
  • \$\begingroup\$ Let's just say it's mostly personal preference (or company policy) whether or not you should still comment on self documenting code. And I'll leave this interesting read for those interested in when/why not to comment code. \$\endgroup\$ – Imus Oct 25 '17 at 8:38
  • \$\begingroup\$ StringBuilder.reverse() \$\endgroup\$ – greybeard Oct 25 '17 at 8:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.