patternjavaMinor
Reverse the character order of the words in a string
Viewed 0 times
theorderreversewordscharacterstring
Problem
I've been trying to learn Java and one way I've been teaching myself is by tackling "programming challenge questions".
One such question is "reverse the character order of words in a string".
For example,
I've written an implementation and I'd like some feedback, particularly regarding the readability and efficiency of the code (basically, is there a slicker implementation of what I'm trying to accomplish - to me, the answer is yes, especially the last part of my implementation).
The implementation is basically doing the following:
`public String reverseWordsInString(final String string) {
final String[] candidateReversedWordsString = new String[1];
candidateReversedWordsString[0] = string;
int stringCharacterIndex = 0;
int wordStartIndex = 0;
final StringBuilder reverseWordStringBuilder = new StringBuilder();
while (stringCharacterIndex 0) {
candidateReversedWordsString[0] = new StringBuilder().
appen
One such question is "reverse the character order of words in a string".
For example,
" I have a hat " => " I evah a tah " (note the spaces). In my implementation, I don't deal with punctuation, so "I have a hat!" => "I evah a !tah".I've written an implementation and I'd like some feedback, particularly regarding the readability and efficiency of the code (basically, is there a slicker implementation of what I'm trying to accomplish - to me, the answer is yes, especially the last part of my implementation).
The implementation is basically doing the following:
- Create a one element-sized String array called
candidateReversedWordsStringand initialize it with the input String
- Create a StringBuilder that will build all the reversed words.
- Iterate through the characters in the string
- If the character is not a space and the StringBuilder is empty, note the character index value in the
wordStartIndexvariable and add the character to the StringBuilder.
- If the character is not a space and the StringBuilder is not empty, add the character to the StringBuilder
- If the character is a space and the StringBuilder is not empty, take the only element in the
candidateReversedWordsStringarray and basically concatenate the element with the reversed word from the StringBuilder using substrings.
- If at the final character and the StringBuilder is not empty, slight adjustment to the concatenation
`public String reverseWordsInString(final String string) {
final String[] candidateReversedWordsString = new String[1];
candidateReversedWordsString[0] = string;
int stringCharacterIndex = 0;
int wordStartIndex = 0;
final StringBuilder reverseWordStringBuilder = new StringBuilder();
while (stringCharacterIndex 0) {
candidateReversedWordsString[0] = new StringBuilder().
appen
Solution
I like the effort that you put into naming your variables. You may have gone a bit overboard, though. For example, instead of
A few mechanical issues:
-
You have…
That pattern would be easier to understand if written as
Here, I would opt for a short variable name
This statement summarizes your strategic error:
Basically, you are rebuilding the entire tentative result string every time you want to reverse a word. That's bad for performance, and also complicates your code.
I think that you would be better off without using any
Consider this solution instead:
candidateReversedWordsString, how about result?A few mechanical issues:
- This could be a
staticmethod, since it relies on no instance state.
- It's pretty funny that you defined a
final String[]of length 1 to store oneStringwith the ability to change its only element. Why not just use a regularStringvariable?
-
You have…
int stringCharacterIndex = 0;
…
while (stringCharacterIndex < string.length()) {
…
stringCharacterIndex++;
}That pattern would be easier to understand if written as
for (int i; i < string.length(); i++) {
…
}Here, I would opt for a short variable name
i, which has the connotation of being a loop counter that is an index. string[i] is easy enough to understand.This statement summarizes your strategic error:
candidateReversedWordsString[0] = new StringBuilder().
append(candidateReversedWordsString[0].substring(0, wordStartIndex)).
append(reverseWordStringBuilder.reverse().toString()).
append(candidateReversedWordsString[0].substring(stringCharacterIndex)).toString();Basically, you are rebuilding the entire tentative result string every time you want to reverse a word. That's bad for performance, and also complicates your code.
I think that you would be better off without using any
StringBuilder, and just using a char[] instead, especially since you know that the result will be exactly the same length as the input.Consider this solution instead:
public static String reverseWords(String string) {
char[] c = string.toCharArray();
int wordStartIndex = -1;
for (int i = 0; i < c.length; i++) {
if (c[i] == ' ') {
// Ignore spaces
continue;
}
if (wordStartIndex < 0) {
// Mark start of word
wordStartIndex = i;
}
if (i + 1 == c.length || c[i + 1] == ' ') {
// Word ends here; reverse it
for (int a = wordStartIndex, b = i; a < b; a++, b--) {
char swap = c[a];
c[a] = c[b];
c[b] = swap;
}
wordStartIndex = -1;
}
}
return new String(c);
}Code Snippets
int stringCharacterIndex = 0;
…
while (stringCharacterIndex < string.length()) {
…
stringCharacterIndex++;
}for (int i; i < string.length(); i++) {
…
}candidateReversedWordsString[0] = new StringBuilder().
append(candidateReversedWordsString[0].substring(0, wordStartIndex)).
append(reverseWordStringBuilder.reverse().toString()).
append(candidateReversedWordsString[0].substring(stringCharacterIndex)).toString();public static String reverseWords(String string) {
char[] c = string.toCharArray();
int wordStartIndex = -1;
for (int i = 0; i < c.length; i++) {
if (c[i] == ' ') {
// Ignore spaces
continue;
}
if (wordStartIndex < 0) {
// Mark start of word
wordStartIndex = i;
}
if (i + 1 == c.length || c[i + 1] == ' ') {
// Word ends here; reverse it
for (int a = wordStartIndex, b = i; a < b; a++, b--) {
char swap = c[a];
c[a] = c[b];
c[b] = swap;
}
wordStartIndex = -1;
}
}
return new String(c);
}Context
StackExchange Code Review Q#112202, answer score: 5
Revisions (0)
No revisions yet.