Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

This is a program which computes the circumference of two circles and gives some information such as the difference between circumferences. I want to know whether the method declarations and method calling in my code is correct. I didn't face any errors.

import java.text.*;
import javax.swing.*;
class Q5Main{
    public static void main(String args[]){
        double smallCircum;
        double largeCircum;
        InputHandler input = new InputHandler();
        Question5 myq = new Question5();

        double a = input.getDouble("Radius for the smaller circle:");

        smallCircum = (myq.start(a));//calculate the small circle circumference


        a = input.getDouble("Radius for the larger circle: ");
        largeCircum = (myq.start(a));//calculate the large circle circumference

        myq.showDetails(smallCircum, largeCircum);


    }


}


class Question5{
    DecimalFormat df = new DecimalFormat("0.000");

    private double circumference;//to store circumference

    //carry out the full process
    public double start(double value){
        return circum(value);
    }




    //compute the circumference
    private double circum(double radius){
        circumference = 2 * Math.PI * radius;
        return circumference;
    }


    public void showDetails(double smallCircum, double largeCircum){
        System.out.println("Circumference of smaller circle: "+df.format(smallCircum));
        System.out.println("\n");
        System.out.println("Circumference of larger circle: "+df.format(largeCircum));
        System.out.println("\n");
        System.out.println("Difference: "+df.format((largeCircum-smallCircum)));

    }

}





class InputHandler{
    private static final String DOUBLE_DEFAULT_PROMPT = "Enter integer: ";

    public double getDouble(){
        return getDouble(DOUBLE_DEFAULT_PROMPT);
    }

    public static double getDouble(String prompt){
        String inStr;
        inStr = JOptionPane.showInputDialog(null,prompt);
        return Double.parseDouble(inStr);
    }
}
share|improve this question
up vote 7 down vote accepted

Although your code works, it's not so good.

Declare variables as late as possible

Here you declare some variables at the top, and use them much later, for example smallCircum:

double smallCircum;
double largeCircum;
InputHandler input = new InputHandler();
Question5 myq = new Question5();

double a = input.getDouble("Radius for the smaller circle:");

smallCircum = (myq.start(a));//calculate the small circle circumference

It would be better to declare as late as possible, right before you need it:

InputHandler input = new InputHandler();
Question5 myq = new Question5();

double a = input.getDouble("Radius for the smaller circle:");
double smallCircum = (myq.start(a));//calculate the small circle circumference

Pointless field

In Question5, the variable circumference doesn't need to be a field. It's only used here:

private double circum(double radius){
    circumference = 2 * Math.PI * radius;
    return circumference;
}

But there is no need to store that value in a field, and no need for even a local variable, you can simply return it directly:

private double circum(double radius) {
    return 2 * Math.PI * radius;
}

static method

It's unclear why this method in InputHandler is static:

public static double getDouble(String prompt){

I suggest to drop the static keyword, make it non-static.

Poor naming

Looking at this:

smallCircum = (myq.start(a));//calculate the small circle circumference

A method named start returns the calculated circumference. That's really not intuitive. Why not rename start to calculateCircum? If its containing Question5 class needs to be kept general purpose and not imply circumference, then perhaps you can rename the method to just calculate instead. Probably almost anything is better than start.

Another thing, a is a poor name to store radius.

Make private what you can

In Question5, df should be private.

Make final what you can

In Question5, df can be final.

Suggested implementation

With the above suggestions applied, the solution becomes:

package com.janosgyerik.practice.oj.leetcode.hard.SubstringWithConcatenationOfAllWords;

class Q5Main {
    public static void main(String args[]) {
        InputHandler input = new InputHandler();
        Question5 myq = new Question5();

        double smallRadius = input.getDouble("Radius for the smaller circle:");
        double smallCircum = myq.calculate(smallRadius);

        double largeRadius = input.getDouble("Radius for the larger circle: ");
        double largeCircum = myq.calculate(largeRadius);

        myq.showDetails(smallCircum, largeCircum);
    }
}    

class Question5 {
    private final DecimalFormat df = new DecimalFormat("0.000");

    public double calculate(double value) {
        return circum(value);
    }

    private double circum(double radius) {
        return 2 * Math.PI * radius;
    }

    public void showDetails(double smallCircum, double largeCircum) {
        System.out.println("Circumference of smaller circle: " + df.format(smallCircum));
        System.out.println("\n");
        System.out.println("Circumference of larger circle: " + df.format(largeCircum));
        System.out.println("\n");
        System.out.println("Difference: " + df.format((largeCircum - smallCircum)));
    }
}    

class InputHandler {
    private static final String DOUBLE_DEFAULT_PROMPT = "Enter integer: ";

    public double getDouble() {
        return getDouble(DOUBLE_DEFAULT_PROMPT);
    }

    public double getDouble(String prompt) {
        String inStr = JOptionPane.showInputDialog(null, prompt);
        return Double.parseDouble(inStr);
    }
}
share|improve this answer

janos gave a great answer. I would add that you should validate user input before calculation to make sure the string they enter is an integer of the size you expect.

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.