Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This simple program uses Scanner to obtain a radius from the user, then calculates and outputs the circumference and area.

Questions:

  1. Is it customary in Java to place a function call inside of a println(), or is it better to put the returned value into a separate variable and then display that?
  2. Should I consider implementing input-validation for practice as this is a toy program?
  3. Am I correct in making the calculation functions static?

import java.util.Scanner;
import java.lang.Math;

public class Circle {

    public static double getCircumference(double radius) {
        return 2 * Math.PI * radius;
    }

    public static double getArea(double radius) {
        return Math.PI * radius * radius;
    }

    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);

        double radius = sc.nextDouble();

        System.out.println("Circumference: ");
        System.out.println(getCircumference(radius));
        System.out.println("Area: ");
        System.out.println(getArea(radius));

        sc.close();
    }
}

Sample output:

5.5
Circumference: 
34.55751918948772
Area: 
95.03317777109123
share|improve this question
2  
1. Could you attach some sample output? 2. Why restrict the radius to int? The class name, Circle, suggests something more general. –  GraniteRobert Jul 16 at 21:31
    
@GraniteRobert: It doesn't have to be restricted; I just thought about an int at first. I can change that, along with providing sample output. –  Jamal Jul 16 at 21:32
    
I wondered if you intended separate lines in the output. I thought I was misunderstanding my JAVA. –  GraniteRobert Jul 16 at 21:41
    
@GraniteRobert: I actually like them on separate lines, but I didn't take the time to find out how to do it. I suppose it's another minor thing anyway. –  Jamal Jul 16 at 21:42
1  
Java needs to catch up with the times and add a Tau constant. tauday.com/tau-manifesto –  RubberDuck Jul 17 at 1:07

3 Answers 3

up vote 13 down vote accepted

Is it customary in Java to place a function call inside of a println(), or is it better to put the returned value into a separate variable and then display that?

This is something I would say is primarily opinion based. For this little app, I think it's fine to put the function method call inside a println.

When it comes to more complex things, it is usually better to do the calculation separately from the output (or most importantly logging), because you don't want your program behavior to change when you remove a System.out.println or logging statement.


Should I consider implementing input-validation for practice as this is a toy program?

I don't see any need to use input-validation when you're using nextInt on the Scanner. Perhaps you'd want to avoid a negative size, and either throw an exception or show an error message to the user, but that's probably it.


Am I correct in making the calculation functions static?

Yes, absolutely. I see no reason for why they should not be static. Given the usefulness of them, it is a good idea to have them public as well (which you have).


Feature-request: You're currently not prompting for any input which might make the user confused. A simple System.out.println("Enter radius:"); would be nice to have.


Overall, your code looks perfectly fine. A minor minor minor nitpick is that you're writing one space more than necessary here:

System.out.println("Circumference: ");
System.out.println("Area: ");

Another minor nitpick is that your Scanner variable could be named either scanner or input instead. (Even though sc is a quite normal variable name for a Scanner) #perfectionist-naming.

share|improve this answer
    
Thanks for the nitpick. ;-) I forgot that this is displayed on separate lines, so that extra space is useless. –  Jamal Jul 16 at 21:38

Creating a Circle object would probably be more idiomatic Java. The code would read a bit more fluently (for example, circle.getCircumference()). It also gives you a good place to validate that the radius is nonnegative.

import java.util.Scanner;

public class Circle {
    private final double radius;

    public Circle(double radius) {
         if (radius < 0) {
             throw new IllegalArgumentException();
         }
         this.radius = radius;
    }

    public double getCircumference() {
        return 2 * Math.PI * radius;
    }

    public double getArea() {
        return Math.PI * radius * radius;
    }

    public static void main(String[] args) {
        try (Scanner sc = new Scanner(System.in)) {
            double radius = sc.nextDouble();
            Circle circle = new Circle(radius);

            System.out.println("Circumference: ");
            System.out.println(circle.getCircumference());
            System.out.println("Area: ");
            System.out.println(circle.getArea());
        }
    }
}

A couple of minor points as well:

  • import java.lang.Math is unnecessary, because java.lang.* are always available already.
  • Since Java 7, try-with-resources gives you a way to automatically close auto-closeable objects like Scanner.
  • Neither your code nor my code above checks that there is a double to read at all. If the input is garbage, then a java.util.InputMismatchException will be thrown, which is unhandled.
share|improve this answer
    
Ah, so I wasn't creating an actual Circle class. I should've known otherwise since this is sort of the same as in C++. It looks like I was actually using free functions. –  Jamal Jul 17 at 0:44
2  
I would guess that for someone with good education and experience, they did whatever they did because of convenient for the purpose at that moment. You are sadly mistaken if you think one person doesn't know how to do things like this. It will be object oriented when it needs to be object oriented and scalable. –  randomA Jul 17 at 7:13

I actually didn't like having each label and result displayed on separated lines, and instead wanted to have each value on separate lines like this:

5.5
Circumference: 34.55751918948772
Area: 95.03317777109123

I've discovered that print() accomplishes this, alongside println() for newlines.

I've also incorporated @Simon's suggestion regarding a user input prompt.

System.out.print("Input radius: ");

// ...

System.out.println();
System.out.print("Circumference = ");
System.out.println(circle.getCircumference());
System.out.print("Area = ");
System.out.print(circle.getArea());

With this, I get the following output (using the same radius):

Input radius: 5.5

Circumference = 34.55751918948772
Area = 95.03317777109123
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.