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.

I am testing a quicksort implementation. Arrays of random size with random data are passed to quicksort and the return time is averaged out. The user is prompted to enter values for

  • minimum length of array
  • maximum length of array
  • maximum value in array
  • minimum value in array
  • if the user wishes to see the sorted and unsorted arrays
  • if the user wishes the program to check itself by testing to make sure the array was sorted properly

I tried breaking up the prompts for user input into several methods but things are getting ugly. Are there any guidelines to follow for when prompting the user for information? One thing I'm not sure about is when to prompt with a question mark vs a colon (e.g. "what is x?" vs "input x:").

public static void main(String[] args)
{
    getInput();
}

public static void getInput()
{
    int bound = getNumber("How many arrays should be generated for quicksort?");
    int minVal = getNumber("Minimum value of element:");
    int maxVal = getNumber("Maximum value of element:");
    int minLength = getNumber("Minimum length of an array:");
    int maxLength = getNumber("Maximum length of an array:");
    boolean giveOutput = yes_no("Would you like to see the sorted and unsorted arrays?");
    boolean checkCorrectness = yes_no("Check proccessed arrays to see if sorted correctly?");

    long totalTime = 0, sumArraySizes = 0;

    for(int i = 0; i < bound; i++)
    {
        int[] sample = generateRandomArray(minLength, maxLength, minVal, maxVal);
        sumArraySizes += sample.length;
        if(giveOutput)
            System.out.println(Arrays.toString(sample));
        long startTime = System.nanoTime();//was using System.currentTimeMillis(); but endTime and startTime would always be the same
        qs(sample, 0, sample.length-1);
        long endTime = System.nanoTime();
        totalTime += endTime-startTime;
        if(giveOutput)
            System.out.println(Arrays.toString(sample)+"\n");
        if(checkCorrectness)
            if(!isSorted(sample))
                System.out.println("WARNING! mistake detected");
    }

    System.out.println("Average size of array: "+sumArraySizes/bound);
    System.out.println("Average time taken to sort array: "+totalTime/bound+" nanoseconds");
}

/*prompts user with a message and returns number they enter*/
public static int getNumber(String message)
{
    Scanner sc = new Scanner(System.in);
    System.out.println(message);
    return sc.nextInt();
}

/*http://stackoverflow.com/questions/15927885/yes-no-with-boolean-or-if-else*/
public static boolean yes_no(String message)
{
    Scanner sc = new Scanner(System.in);
    String answer;
    boolean yn;

    System.out.print(message);
    OUTER:
    while (true) {
        answer = sc.nextLine().trim().toLowerCase();
        switch (answer) {
            case "y":
                yn = true;
                break OUTER;
            case "n":
                yn = false;
                break OUTER;
            default:
                System.out.println("Please answer with y or n");
                break;
        }
    }
    return yn;
}

What should the method yes_no be named? I wanted to call it yesOrNo but that looked strange.

share|improve this question
add comment

2 Answers 2

Spaces

Put a space after for and if:

  • for(int i = 0; i < bound; i++) --> for (int i = 0; i < bound; i++)
  • if(giveOutput) --> if (giveOutput)

Put a space around a -:

sample.length-1 --> sample.length - 1

And the same goes for +:

System.out.println("Average time taken to sort array: "+totalTime/bound+" nanoseconds");

becomes:

System.out.println("Average time taken to sort array: " + totalTime/bound + " nanoseconds");

if-statements

Add braces around if-statements:

if (giveOutput) {
    System.out.println(Arrays.toString(sample));
}

Avoid nested if-statements such as:

if(checkCorrectness)
    if(!isSorted(sample))
        System.out.println("WARNING! mistake detected");

Change that to:

if (checkCorrectness && !isSorted(sample)) {
    System.out.println("WARNING! mistake detected");
}

(Consider throwing an exception for this by the way. A mistake is important stuff)

Naming

Whatever qs is doing (A quicksort I guess), the name is too short to be meaningful. Name it quicksort instead.

Name your methods for inputting things as input*:

  • getNumber --> inputNumber
  • yes_no --> inputBoolean

Are there any guidelines to follow for when prompting the user for information? One thing I'm not sure about is when to prompt with a question mark vs a colon (e.g. "what is x?" vs "input x:").

I believe this can be answered better on User Experience. They probably have a question about it already.

I would think that a question mark should be used when asking a question, and a colon should be used when encouraging the user to enter something. Of course this doesn't say much though as all your user prompts can be modified to the other form. My opinion is that what you currently have is good though.

Other comments

  • Good choice to use System.nanoTime();

  • To make the existence of another line in your output clear, I would change System.out.println(Arrays.toString(sample)+"\n"); to:

    if (giveOutput) {
        System.out.println(Arrays.toString(sample));
        System.out.println();
    }
    
  • In Java, the convention is that { goes on the same line, not on it's own line.

    public static int getNumber(String message)
    {
    

    should be, according to the conventions:

    public static int getNumber(String message) {
    
  • Avoid multiple variable declarations on the same line

    long totalTime = 0, sumArraySizes = 0;
    

    can be:

    long totalTime = 0;
    long sumArraySizes = 0;
    
share|improve this answer
    
Regaurding the spacing could you argue the point why there should be more? For example was a study done to find putting a space between for and ( makes it more readable? Compared to how my code was when I first started programing I add many more spaces, however I personally notice spacing can be over done. –  Celeritas Jun 18 at 2:32
    
@Celeritas Excellent question! The only argument I have is that it is the Java conventions. Also take a look at Oracle's examples and you will see that they use more spaces than you. Of course you shouldn't overdo it, it's enough with one space around the operators and keywords. –  Simon André Forsberg Jun 18 at 11:02
    
"In Java, the convention is that { goes on the same line, not on it's own line." according to whom? I was taught putting the brace on its own line is more readable. –  Celeritas Jun 18 at 18:37
    
@Celeritas Again, according to the conventions. In C# the convention is to put it on its own line, but not in Java. –  Simon André Forsberg Jun 18 at 19:01
add comment

I'm not sure if you need all of that in yes_no(). The top answer to the commented question gives something a bit shorter (also no OUTER), and another states that you can just return true if the user inputs 'y', otherwise return false. I'd probably go with the latter, unless you really insist on getting valid feedback from the user.

share|improve this answer
    
@Jamal I'm not sure if I'm understanding you fully. If I didn't have a function such as yes_no() then I would need more code somewhere else, and it gets called twice. I don't think I understand what you mean by hardcode. –  Celeritas Jun 18 at 2:35
    
@Celeritas: I'm saying that it can be written shorter. –  Jamal Jun 18 at 2:37
add comment

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.