Problem
Prompt:
You have been given a string s, which is supposed to be a sentence.
However, someone forgot to put spaces between the different words, and
for some reason, they capitalized the first letter of every word.
Return the sentence after making the following amendments:Put a single space between the words. Convert the uppercase letters to
lowercase.
Any suggestions on my code?
String amendTheSentence(String s) {
StringBuilder modifiedString = new StringBuilder();
char[] sToArray = s.toCharArray();
if (Character.isUpperCase(sToArray[0])) {
modifiedString.append(Character.toLowerCase(sToArray[0]));
} else {
modifiedString.append(sToArray[0]);
}
for (int i = 1; i < sToArray.length; i++) {
char currentChar = sToArray[i];
if (Character.isUpperCase(sToArray[i])) {
currentChar = Character.toLowerCase(currentChar);
modifiedString.append(" ");
}
modifiedString.append(currentChar);
}
return modifiedString.toString();
}
Solution
Thanks for sharing the code.
This is quite good for a first attempt (except the bug).
Things to consider:
Odd ball solution
This means that you do the same thing in different ways.
-
use of variable vs direct array access
when you handle the first letter you directly access it using array index access
sToArray[0]
whereas for all the other letters you first put the actual char into a local variablecurrentChar
but you do not use it in theif
statement. You should do the same at all places. -
similar logic that could be the same
this is a bit trickier and may need some experience, but:
The difference between the handling of the first letter and all the other letters is that you don’t put a space before it. Having in mind that there is an empty string (""
) in Java putting that instead of the space is just a configuration, a different value to use and not a different behavior needing different instructions (or their order). Including point 1 your code could look like this:
(I intentionally kept your bug!)String amendTheSentence(String s) { StringBuilder modifiedString = new StringBuilder(); char[] sToArray = s.toCharArray(); String separator = ""; char currentChar = sToArray[0]; if (Character.isUpperCase(currentChar)) { currentChar = Character.toLowerCase(currentChar); modifiedString.append(separator ); } modifiedString.append(currentChar); for (int i = 1; i < sToArray.length; i++) { String separator = " "; char currentChar = sToArray[i]; if (Character.isUpperCase(currentChar)) { currentChar = Character.toLowerCase(currentChar); modifiedString.append(separator); } modifiedString.append(currentChar); } return modifiedString.toString(); }
Now you have 4 lines which are repeated exactly the same before the loop and inside the loop. This 4 lines can be extracted to a parameterized method:
private void convertAndAppendTo(StringBuilder modifiedString, char currentChar, String separator){ if (Character.isUpperCase(currentChar)) { currentChar = Character.toLowerCase(currentChar); modifiedString.append(separator ); } modifiedString.append(currentChar); }
and your original method changes to this:
String amendTheSentence(String s) { StringBuilder modifiedString = new StringBuilder(); char[] sToArray = s.toCharArray(); convertAndAppendTo(modifiedString, sToArray[0], ""); for (int i = 1; i < sToArray.length; i++) { convertAndAppendTo(modifiedString, sToArray[i], " "); } return modifiedString.toString(); }
Magic numbers
In your code the 0
is a magic number and should be declared as a constant with a meaningful name like this:
private static final int FIRST_LETTER_INDEX = 0;
String amendTheSentence(String s) {
StringBuilder modifiedString = new StringBuilder();
char[] sToArray = s.toCharArray();
convertAndAppendTo(modifiedString, sToArray[FIRST_LETTER_INDEX], "");
// ...
I for myself would also tread literal strings as magic numbers and convert them into constants:
private static final int FIRST_LETTER_INDEX = 0;
private static final String FIRST_SEPARATOR = "";
private static final String INTER_WORD_SEPARATOR = " ";
String amendTheSentence(String s) {
StringBuilder modifiedString = new StringBuilder();
char[] sToArray = s.toCharArray();
convertAndAppendTo(modifiedString, sToArray[FIRST_LETTER_INDEX], FIRST_SEPARATOR);
// ...
Naming
Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.
Get your names from the problem domain, not from the technical solution.
You have a variable sToArray
that stores the individual chars. The name tells something about the type of the variable but it should tell something about its purpose instead. So a better name might be individualLetters
or just letters
.
/**
* Fixed the sentence by putting a separator before an upper case character. Then, convert all characters to lowercase.
* @param unseparatedString - the string to convert
* @param separator - the words divider
* @return the sentence that is divided into words.
*/
private String parseStringToSentence(String unseparatedString, char separator) {
StringBuilder stringBuilder = new StringBuilder();
char[] characters = unseparatedString.toCharArray();
for(char character : characters) {
if(Character.isUpperCase(character)) {
stringBuilder.append(separator);
}
stringBuilder.append(character);
}
return stringBuilder.toString().toLowerCase().trim();
}
- Always put a documentation comment on methods and classes. Your
program might be short now but when it grew, it will lead to
confusion. - Make sure your naming of variable is descriptive.
- For this program, since I am just accessing the array consecutively, it is better to use a foreach loop. It’s short and
simple. - Since you will be converting all the characters into lower case, you can just simply use the
String.toLowerCase()
method on the end
rather than putting it on anif
statement. - I trim the final sentence to remove the extra space in the front.