Questions & Responses: Let me tell you about you

Question

Please review this code.

import java.util.Scanner;
import java.util.* ;
public class LMComplex {
Scanner name  = new Scanner(System.in);
Scanner age  = new Scanner(System.in);
Scanner gender  = new Scanner(System.in);
Scanner height  = new Scanner(System.in);
String usrname;
String usrage;
String usrgender;
Double usrheight;
public void main(){
System.out.println("Enter your name: ");
usrname = name.nextLine();
System.out.println("Enter your age(Numbers): ");
usrage = age.nextLine();
System.out.println("Enter your gender(male/female): ");
usrgender = gender.nextLine();
System.out.println("Enter your height(feet.inches . 5 feet 5 inches = 5.5): ");
usrheight = height.nextDouble();
Double rusrheight = Math.ceil(usrheight);
if(rusrheight > usrheight)
{
rusrheight = rusrheight - 1;
}
else 
{

}
Double rusrheightininches = usrheight - rusrheight;
if(rusrheightininches >= 0.10 && rusrheightininches < 0.12)
{
rusrheightininches = rusrheightininches * 100;
} else {
rusrheightininches = rusrheightininches * 10;
}
Double rusrheightinincheschecker = rusrheightininches;
rusrheightininches = Math.ceil(rusrheightininches);
if(rusrheightininches > rusrheightinincheschecker)
{
rusrheightininches = rusrheightininches - 1;
}
else 
{

}
System.out.println("Hii " + usrname + ". I am Cartic, a basic textual virtual personal assistant. I do know that you are " + usrage +" years old. You are a " + usrgender + " and are " + rusrheight + " feet " + rusrheightininches + " inch(es) tall");
}
}

Answer 1

Indenting is the first part of common practice that you should consider when sharing your code with anyone. Here is your code, which I have put though the 'standard' indenting function of Eclipse:

import java.util.Scanner;

public class LMComplex {
    Scanner name = new Scanner(System.in);
    Scanner age = new Scanner(System.in);
    Scanner gender = new Scanner(System.in);
    Scanner height = new Scanner(System.in);
    String usrname;
    String usrage;
    String usrgender;
    Double usrheight;

    public void main() {
        System.out.println("Enter your name: ");
        usrname = name.nextLine();
        System.out.println("Enter your age(Numbers): ");
        usrage = age.nextLine();
        System.out.println("Enter your gender(male/female): ");
        usrgender = gender.nextLine();
        System.out.println("Enter your height(feet.inches . 5 feet 5 inches = 5.5): ");
        usrheight = height.nextDouble();
        Double rusrheight = Math.ceil(usrheight);
        if (rusrheight > usrheight)
        {
            rusrheight = rusrheight - 1;
        }
        else
        {

        }
        Double rusrheightininches = usrheight - rusrheight;
        if (rusrheightininches >= 0.10 && rusrheightininches < 0.12)
        {
            rusrheightininches = rusrheightininches * 100;
        } else {
            rusrheightininches = rusrheightininches * 10;
        }
        Double rusrheightinincheschecker = rusrheightininches;
        rusrheightininches = Math.ceil(rusrheightininches);
        if (rusrheightininches > rusrheightinincheschecker)
        {
            rusrheightininches = rusrheightininches - 1;
        }
        else
        {

        }
        System.out.println("Hii " + usrname + ". I am Cartic, a basic textual virtual personal assistant. I do know that you are " + usrage
                + " years old. You are a " + usrgender + " and are " + rusrheight + " feet " + rusrheightininches + " inch(es) tall");
    }
}

Now, it is much easier to see the structure.

Java code-style guidelines put the { opening brace on the same line as the control statement, so, for example, the code:

        if (rusrheightininches >= 0.10 && rusrheightininches < 0.12)
        {
            rusrheightininches = rusrheightininches * 100;
        } else {
            rusrheightininches = rusrheightininches * 10;
        }

should be:

        if (rusrheightininches >= 0.10 && rusrheightininches < 0.12) {
            rusrheightininches = rusrheightininches * 100;
        } else {
            rusrheightininches = rusrheightininches * 10;
        }

Also, in the previous if statement you have an empty else block.... you can just remove it:

        if (rusrheight > usrheight)
        {
            rusrheight = rusrheight - 1;
        }
        else
        {

        }

becomes:

        if (rusrheight > usrheight) {
            rusrheight = rusrheight - 1;
        }

As for your variables, they are nice descriptive names, but, in Java, it is common to use what is called 'camelCase', where the first letter is lower-case, and the subsequent first-word-letters are Capitalized.

Also, while it is often convenient to shorten long parts of variables, the abbreviation of 'user' to 'usr' is not saving you much.....

So, for example, your variables :

String usrname;
String usrage;
String usrgender;
Double usrheight;

would be

String userName;
String userAge;
String userGender;
Double userHeight;

I would recommend that you use an IDE (Eclipse/IntelliJ/Netbeans) to help you get these things right. There is some debate about whether new programmers should use an IDE or not, because they make some things really easy and you may miss understanding some of the basic requirements of the language.... but, I believe that the IDE's allow you to focus on the stuff that is more important, like the code content.

Answer 2

Scanner name  = new Scanner(System.in);
Scanner age  = new Scanner(System.in);
Scanner gender  = new Scanner(System.in);
Scanner height  = new Scanner(System.in);

There is absolutely no reason to have four scanner variables. It does not matter which one you use, it would be better to just have one: Scanner scanner = new Scanner(System.in);

Also, you should close the Scanner once your done with it by calling scanner.close(); (or use try-with-resources statement)

---

Double usrheight;

You don't need to use Double, use double instead. The difference is that double is a primitive type, which means that among other things it cannot be set to null.

---

else 
{

}

There's no reason in including an else if you don't do anything in it. Remove that part.

---

A variable name such as rusrheightininches is easier to read by using "camelCase", I would name it realUserHeightInches

Instead of using rusrheightininches = rusrheightininches * 100; you can do rusrheightininches *= 100; which is just a shorter way of writing it.

---

Variable Scope. Your variables are currently declared as fields in the class. They only need to be accessible inside one method, so you can declare them within that method as local variables.

---

Taking it further

Java is an Object Oriented language. Write your own classes, use objects, add more methods. Everything does not need to be done inside one method. I suggest you read Oracle's tutorial at http://docs.oracle.com/javase/tutorial/java/javaOO/ . You could use a Person class for example with the properties name, age, gender height.

Answer 3

This code does not work.

When you try to run your program, Java will complain because it can't find the main method. Isn't there a main method right there? Yes, but it has the wrong signature. Java looks for a public static void main (String[] argv) – both the static and the String[] argument are important.

The simplest way to add that is

public static void main(String[] argv) {
    new LMComplex().main();
}

The next big problem is that you are creating multiple Scanners around the same input stream System.in. Due to buffering, lines will get lost. Instead, create only one Scanner, and request multiple nextLines from it:

Scanner in = new Scanner(System.in);
System.out.println("Enter your name: ");
usrname = in.nextLine();
System.out.println("Enter your age(Numbers): ");
usrage = in.nextLine();
...

Once these issues are fixed, the program will actually start to work correctly, and there are many style issues to improve (see rolfl's answer for a good summary).

Another bug: The nextDouble method of a Scanner is locale dependent. When I enter 5.5 as the height, I will get an exception because the decimal separator in my German locale is the comma – 5,5 works perfectly. To avoid this, we need to set a locale that works independently of the system locale:

Scanner in = new Scanner(System.in);
in.useLocale(new Locale("en", "US"));

Answer 4

Naming:

You should use readable names for your variables. Furthermore the convention for Java-naming is camelCase. Your fieldnames should start with a lower-case letter and each new "word" inside of these names, should start with a single upper-case letter.

readable here means, you shouldn't have to do some crazy tounge-exercise when reading variable names out loud. usr and similar is not good, there is no need to save characters in the wrong place:

usrname --> userName
usrage  --> userAge
//repeat with all other variables

Answer 5

Most has been said already but I'll add that you can contract these statements

rusrheightininches = rusrheightininches * 100; 
rusrheightininches = rusrheightininches - 1;

to

rusrheightininches *= 100;
rusrheightininches -= 1;

I'll restate the importance of naming conventions: this is unreadable

rusrheightinincheschecker 

but this isn't:

rUserHeightInInchesChecker

Answer 6

Your height input routine is broken, because you have used a floating-point number to represent two numbers that just happen to be separated by a dot. The fundamental flaw is that 5 ft. 1 in. is indistinguishable from 5 ft. 10 in.

System.out.println("Enter your height(feet.inches . 5 feet 5 inches = 5.5): ");
usrheight = height.nextDouble();
Double rusrheight = Math.ceil(usrheight);
if(rusrheight > usrheight)
{
rusrheight = rusrheight - 1;
}
else 
{

}
Double rusrheightininches = usrheight - rusrheight;
if(rusrheightininches >= 0.10 && rusrheightininches < 0.12)
{
rusrheightininches = rusrheightininches * 100;
} else {
rusrheightininches = rusrheightininches * 10;
}
Double rusrheightinincheschecker = rusrheightininches;
rusrheightininches = Math.ceil(rusrheightininches);
if(rusrheightininches > rusrheightinincheschecker)
{
rusrheightininches = rusrheightininches - 1;
}
else 
{

}

There are a lot of puzzling moments as I read through that code.

• Why are there empty else clauses? Just leave them out.
• Why do you declare rusrheight, rusrheightininches, and rusrheightinincheschecker as Double objects? That will cause much unnecessary autoboxing and unboxing.
• What does rusrheight mean? Isn't it just Math.floor(usrheight)?
• What is the point of rusrheightinincheschecker? It just helps to compute Math.floor(rusrheightininches) the hard way.

In the end, though, those concerns are irrelevant since the parsing strategy is fundamentally flawed. You really ought to write a custom parsing routine. For example:

/**
 * Reads a height in feet and inches, in the format fff.ii.
 * The input must be at the end of a line.
 *
 * @return The height as inches (i.e., "4.3" is interpreted as 4 * 12 + 3 = 51).
 */
public static double readHeightAsInches(Scanner scan)
        throws InputMismatchException {
    scan.next(Pattern.compile("([+-]?\\d+)(?:\\.(\\d*\\.?\\d*))?$"));
    MatchResult match = scan.match();
    int feet = Integer.parseInt(match.group(1));
    double inches = (match.group(2) != null && !match.group(2).isEmpty()) ?
        Double.parseDouble(match.group(2)) : 0.0;
    return 12 * feet + (feet >= 0 ? inches : -inches);
}

It's easier to return a single number representing the number of inches rather than a pair of numbers representing feet and inches.

That would be paired with a formatting routine:

public static String formatHeight(double inches) {
    StringBuffer sb = new StringBuffer(20);
    if (inches < 0) {
        sb.append('-');
        inches = -inches;
    }
    if (inches >= 24) {
        sb.append((int)inches / 12).append(" feet ");
    } else if (inches >= 12) {
        sb.append((int)inches / 12).append(" foot ");
    }
    sb.append(inches %= 12);
    sb.append(inches >= 1 && inches < 2 ? " inch" : " inches");
    return sb.toString();
}