# Separate combined words in a sentence and convert them into Pig Latin

My program is complete and working, but I would like second opinions on it.

Here are the instructions for the assignment:

1. The program will accept a string as input in which all of the words are run together, but the first character of each word is uppercase. Convert the string to a string in which the words are separated by spaces and only the first word starts with an uppercase letter. For example, the string "StopAndSmellTheRoses" would be converted to "Stop and smell the roses".

2. Then the program will convert each word in the result string of task 1 into "Pig Latin". In one version of Pig Latin, you convert a word by removing the first letter, placing that letter at the end of the word, and then appending "ay" to the word.

For example, for the result string "Stop and smell the roses" in task 1, the Pig Latin string should be "topSay ndaay mellsay hetay osesray".

Requirements:

• Your program should have 3 methods: a main method, a method for task 1, and a method for task2.
• The method for task 1 should return the result string of task1.
• The method for task 2 should display the Pig Latin string.

import java.util.Scanner;
public class Assignment9
{
public static void main(String[] args)
{
// Variables
String sentence, revisedSentence, latin = " ";

// Create a Scanner object for keyboard input
Scanner keyboard = new Scanner(System.in);

// Get the input string
System.out.print("Enter sentence: ");
sentence = keyboard.nextLine();

//Close keyboard
keyboard.close();

revisedSentence = WordSeparator(sentence);

System.out.print("Revised Sentence: " +revisedSentence);

PigLatin(revisedSentence, latin);

System.exit(0);
}

private static String WordSeparator(String sentence)
{
// Variables
StringBuilder str = new StringBuilder(sentence);
int i = 1;

// While loop repeats until the end of the sentence
while (i < str.length())
{
// Checks for upper case characters
if(Character.isUpperCase(str.charAt(i)))
{
//Inserting space
str.insert(i, ' ');
char ch = Character.toLowerCase(str.charAt(i+1));
str.setCharAt((i+1), ch);
}
i++;
}
return str.toString();
}

private static void PigLatin(String revisedSentence, String latin)
{
// Split sentence by spaces
String[] tokens = revisedSentence.split(" ");

// Store all the words into an array
String[] arr = new String[tokens.length];

// Convert English sentence into Pig Latin
for (int i = 0; i < tokens.length; i++)
{
// Get string from array
String str = tokens[i];

// Get first letter from string
String str1 = str.substring(0, 1);

// Get substring from string
String str2 = str.substring(1, str.length());

// Concatenate the two strings in a required format
str2 = str2.concat(str1);

// Concatenate the result and "AY"
str2 = str2.concat("ay");

// Store all of the strings into an array
arr[i] = str2;

// Make a sentence with all the words
latin = latin.concat(str2 + " ");
}

// Display pig latin verison
System.out.println("\nPig Latin Version:" +latin );
}

}


This is in addition to what @thecoder16 already said.

### WordSeparator

Methods should be named using verbs instead of nouns. They usually take some action on the input, for example this method separates the words that were stuck together. So separateWords would be better.

Also, this part is not great:

    // Checks for upper case characters
if(Character.isUpperCase(str.charAt(i)))
{
//Inserting space
str.insert(i, ' ');
char ch = Character.toLowerCase(str.charAt(i+1));
str.setCharAt((i+1), ch);
}


Objections:

1. The str.charAt(i) and str.charAt(i+1) refer to the same character. This is repetitive, and can be prone to errors, as you have to remember to use i+1 the second time because of the inserted space

2. The comments only state the obvious. Unnecessary comments are like noise, and a waste of time to read. Let the code speak for itself.

It would be better this way:

    char ch = str.charAt(i);
if (Character.isUpperCase(ch)) {
str.insert(i, ' ');
str.setCharAt(i + 1, Character.toLowerCase(ch));
}


### PigLatin

The biggest issue here is the latin = latin.concat(...) call for two reasons:

1. String concatenation is inefficient. You should use a StringBuilder like you did in your other method.
2. Reusing input parameters is not a good practice, as it can be confusing and error prone. It would be better to name local variables differently.

Issues similar to WordSeparator:

• The method is poorly named. Perhaps toPigLatin would be better.
• The comments are unnecessary noise

Other smaller issues:

• Instead of printing output, it would be better to return the result
• The String[] arr is unused, so it would be better to remove it
• The counting for loop can be converted to an enhanced loop, for example: for (String str : tokens) { ... }
• The input parameter revisedSentence carries information that's unnecessary for the functioning of this method. Just sentence would be enough.
• Hi, janos. Thank you for your kind input. In your Other smaller issues comments, you said that the variable sentence is enough, but when I replaced revisedSentence with sentence in the method toPigLatin, I ran the program and got incorrect input. So does that mean I should keep the revisedSentence in the method? – user21563966 Apr 21 '15 at 22:14
• +1 for the obvious comments. Better use descriptive names for program elements. About the string concatenation. I would leave that to the compiler to decide, as according to the specs (docs.oracle.com/javase/specs/jls/se8/html/…) it may optimize the + operator. It used to be better to do that in code, but that's no longer necessary. – Kwebble Apr 21 '15 at 22:46
• Thanks @Kwebble, I didn't know that feature of Java 8 – janos Apr 22 '15 at 6:25

I just have two suggestions:

Function names in java should be camelCase, and should say what the function does, like toPigLatin and separateWord instead of WordSeparator and PigLatin.

Also, your pig latin function should return the string in Pig Latin and have main() print it out. I feel like that is better organization.

Otherwise, you have great, working, efficient code!

You are checking the capital letters twice in your loop, you should increment i by 1 when you add in a space. This will also mean you don't need to use i + 1 to de-capitalise the current letter

if(Character.isUpperCase(str.charAt(i)))
^== (i == 3)

str.insert(i, ' ');
^=== (i == 3)

char ch = Character.toLowerCase(str.charAt(i+1));
str.setCharAt((i+1), ch);
^=== (i == 3) + 1
i++;
^=== (i == 4)

new while loop iteration
^=== (i == 4)


change the code in the if this

str.insert(i, ' ');
i++;
char ch = Character.toLowerCase(str.charAt(i));
str.setCharAt(i, ch);

• Do I place this in my if statement in the while loop? If I do that, I will have to declare the char ch in the variables comment section, right? – user21563966 Apr 23 '15 at 3:30
• Yes, in its current spot in the if statement, you are making the string longer by 1 each time you add a space. This will put all the character indexs after and including the currently being read one up by one. You add one to i to account for that, effectively leaving your i referring to the same letter it was before adding the space the normal i++ still happens moving onto the next character – Gilsham Apr 23 '15 at 4:11
• I implement it and the code won't complete. It will prompt me to enter a sentence and then stop. – user21563966 Apr 23 '15 at 4:59
• I'm not sure how it could do that, all it is is adding i++ after the str.insert and removing the + 1 on the is below it – Gilsham Apr 27 '15 at 20:27

Just two pointers for your code:

1. You don't need to do an explicit System.exit(0) at the end of your main() method.
2. A school of thought says that "comments should explain 'why', not 'how'". This mindset encourages developers to write self-documenting code (such as using proper field names, and coding in a more direct way) instead of relying on comments, which may become outdated, and then ultimately misleading. Your choice of comments is unfortunately needless, as practically all the lines are pretty much self-explanatory. Hence, my suggestion here is to remove them and the extra newlines between each line of code. :)

Now, I'm not sure if you can use Java 7 (so that you can rely on try-with-resources for your Scanner object) or even Java 8 (so that you can think of your operations as more of a pipe-lined process). Assuming both are available at your disposal and you are also allowed to sprinkle a little of regex magic, then you may want to consider the following simplification.

public class PigLatin {

public static void main(String[] args) {
try (final Scanner keyboard = new Scanner(System.in)) {
System.out.println("Enter sentence: ");
Stream.of(keyboard.nextLine())
.map(PigLatin::sentenceParser)
.peek(System.out::println)
.forEach(PigLatin::toPigLatin);
}
}

private static String parseSentence(final String input) {
final String result = input.replaceAll("(\\p{Ll})(\\p{Lu})", "$1$2");
return result.substring(0, 1) + result.substring(1).toLowerCase();
}

private static void toPigLatin(final String input) {
Stream.of(input.split(" ")).forEach(i ->
System.out.printf("%s%say ",
i.substring(1), i.substring(0, 1)));
}
}

1. try-with-resources handles the eventual closing of the Scanner object implicitly
2. From the user input, I 'map' it using the required transformation of your task 1
• The regex is used here to insert the extra space before every uppercase character
• Then, I keep the first character of the sentence and convert the rest to lower case
3. Before I start task 2, I 'peek' at the result of task 1 by printing it (System.out::println is a method reference, i.e. equivalent to writing something like System.out.println(<output of task 1>))
4. Finally, I start processing with task 2 'forEach' of the output of task 1 (which in this case is a single sentence)
• I split it on the " " character, and instead of a slightly verbose String concatenation, I rely on formatting the input String given the requirements:
• 'removing the first letter': i.substring(1)
• 'placing that letter at the end of the word': i.substring(0, 1)
• 'and then appending "ay" to the word': "%s%say"
• i think your code is too complicated and hard too understand for the OP. Personally, even I find it a little hard to follow. – Quintec Apr 30 '15 at 0:10
• @thecoder16 is it the stream part, the regex, or the String formatting in the last method (or 'step')? – h.j.k. Apr 30 '15 at 0:25
• The regex and the peek method—I have never seen it before. Also, why a double colon? – Quintec Apr 30 '15 at 0:27
• peek() is a method that allows an intermediate operation to be performed on each stream element, and in my case I am printing it out. As explained in my answer, :: is a method reference. Just some new features of Java 8. :) – h.j.k. Apr 30 '15 at 0:31
• Hmm…interesting! What about the regex? – Quintec Apr 30 '15 at 0:33