Converting the first and last name to pig latin

The following question was taken from Absolute Java 5th ed. by Walter Savitch:


Write a program that starts with the string variable first set to your first name and the string variable last set to your last name. Both names should be all lower- case. Your program should then create a new string that contains your full name in pig latin with the first letter capitalized for the first and last name. Use only the pig latin rule of moving the first letter to the end of the word and adding “ay.” Output the pig latin name to the screen. Use the substring and toUpperCase methods to construct the new name.

For example, given

first = "walt";

last = "savitch";

the program should create a new string with the text “Altway Avitchsay” and print it.

This is the code that I have written:

```
public class Question3 {

private static String first;
private static String last;
private static String newName;

public static void main(String[] args) {

Question3 name = new Question3("walt", "savitch");

System.out.println(name.convertName(first) + " "
+ name.convertName(last));

}

public Question3(String lowerCaseFirstName, String lowerCaseLastName) {
first = lowerCaseFirstName;
last = lowerCaseLastName;
}

private String convertName(String originalName) {
String firstLetter = originalName.substring(0, 1);
newName = nameWithoutFirstLetter(originalName) + firstLetter; // move first letter to the end
newName = capitalizeFirstLetter() + nameWithoutFirstLetter(newName) // capitalize first letter and add "ay" to the end
+ "ay";
return newName;
}

private String capitalizeFirstLetter() {
String capitalFirstLetter = newName.substring(0, 1).toUpperCase();
return capitalFirstLetter;
}

private String nameWithoutFirstLetter(String name) {
String restOfName = name.substring(1);
return restOfName;

Structure

Try not to use static variables. static is for when the value of a field is the same across all instances of your object. But this isn't the case with peoples names.

What if you have two people? You would create your people:

Question3 name = new Question3("walt", "savitch");
    Question3 anotherName = new Question3("john", "smith");

But then, you cannot use them the way you want.

If instead you would rewrite your code like this:

public class Question3 {

    private String first;
    private String last;
    private String newName;

    public static void main(String[] args) {

        Question3 name = new Question3("walt", "savitch");
        Question3 anotherName = new Question3("john", "smith");
        System.out.println(name.convertFullName());
        System.out.println(anotherName.convertFullName());

    }

    public Question3(String lowerCaseFirstName, String lowerCaseLastName) {
        first = lowerCaseFirstName;
        last = lowerCaseLastName;
    }

    public String convertFullName() {
        newName = convertName(first) + " " + convertName(last);
        return newName;
    }

    [...]
}

You can see that you can create however many people you want.

Temporary variables

If you don't need variables, you shouldn't create them. For example:

String restOfName = name.substring(1);
    return restOfName;

could just be

return name.substring(1);

Functions

You have a function which removes the first letter from a string, but you do not have one for getting the first letter of a string, but instead write that functionality directly in your convertName function. This seems inconsistent. I would either have a function for both or neither.

I also wouldn't operate on newName directly in capitalizeFirstLetter. None of your other functions (for example nameWithoutFirstLetter) do this, so it is inconsistent and can be confusing.

Also, right now, newName is just a temporary variable, which isn't ideal. You can never be sure what state it has. I would either change the code so it always holds the complete new name (for example Altway Avitchsay), or just get rid of newName completely.

StackExchange Code Review Q#62424, answer score: 7