3
\$\begingroup\$

Write a program that reads two integers start and end from the user and prints the number from start to end inclusive. However, If end was less than start than you should print them in descending order.

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

    int start = scan.nextInt();
    int end = scan.nextInt();

    if(start < end)
        for(int i = start; i <= end; i++)
            System.out.print(i + " ");

    else if(start > end)
        for(int i = start; i >= end; i--)
            System.out.print(i + " ");  
}
\$\endgroup\$
4
  • \$\begingroup\$ It looks like you have an extra brace at the end (possibly from your upper class declaration), did you mean to include that? \$\endgroup\$
    – Phrancis
    Oct 23, 2015 at 12:49
  • \$\begingroup\$ yh it's from the upper class declaration. The online judge gave me a 47/100, so I'm trying to figure out what's wrong or missing. \$\endgroup\$
    – Kj45
    Oct 23, 2015 at 12:52
  • 1
    \$\begingroup\$ Why don't you go ahead and add in your upper class declaration for completeness. \$\endgroup\$
    – Phrancis
    Oct 23, 2015 at 12:54
  • \$\begingroup\$ That's not exactly the problem now. I sent the complete code to the judge. \$\endgroup\$
    – Kj45
    Oct 23, 2015 at 13:03

5 Answers 5

5
\$\begingroup\$

Input checking

The most obvious issue that I see is that you are not verifying that the correct input is being entered. For example, if you enter a String instead of an int:

> hello
Exception in thread "main" java.util.InputMismatchException
  at java.util.Scanner.throwFor(Scanner.java:864)
  at java.util.Scanner.next(Scanner.java:1485)
  at java.util.Scanner.nextInt(Scanner.java:2117)
  at java.util.Scanner.nextInt(Scanner.java:2076)
  at Main.main(Main.java:8)

A natural way to do this is with Scanner.hasNextInt(). Also, consider adding a little bit of feedback so it is less confusing for the user. Something like this, for example:

    Scanner scan = new Scanner(System.in);
    int start;
    int end;

    System.out.println("Enter 1st whole number: ");
    while (!scan.hasNextInt()) {
        System.out.println("Input must be a whole number. Try again:");
        scan.next();
    }
    start = scan.nextInt();

    System.out.println("Enter 2nd whole number: ");
    while (!scan.hasNextInt()) {
        System.out.println("Input must be a whole number. Try again:");
        scan.next();
    }
    end = scan.nextInt();

Result:

Enter 1st whole number: 
> hello
Input must be a whole number. Try again:
> 5
Enter 2nd whole number: 
> world
Input must be a whole number. Try again:
> 20
5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20

One other (more general) method (though it is more of an anti-pattern in this particular case) to validate things is with a try/catch block. See this answer on Stack Overflow for an example on how to do this.

    Scanner scan = new Scanner(System.in);
    int start;
    int end;

    try { 
        System.out.println("Enter 1st number:");
        start = scan.nextInt();
        System.out.println("Enter 2nd number:");
        end = scan.nextInt();

    } catch (java.util.InputMismatchException e) {
        System.out.println("Input must be an integer.");
        System.out.println("Exiting program.");
        return;
    }
Enter 1st number:
> hello
Input must be an integer.
Exiting program.

Formatting

One other remark, although it is more stylistic, it is often considered a good habit in general to include braces in Java even when they are not needed. So this bit:

if(start < end)
    for(int i = start; i <= end; i++)
        System.out.print(i + " ");

else if(start > end)
    for(int i = start; i >= end; i--)
        System.out.print(i + " ");

Would look like this:

    if(start < end) {
        for(int i = start; i <= end; i++) {
            System.out.print(i + " ");
        }
    }
    else if(start > end) {
        for(int i = start; i >= end; i--) {
            System.out.print(i + " ");  
        }
    }
\$\endgroup\$
2
  • 3
    \$\begingroup\$ I'd say that validating user input with try...catch is an anti-pattern, at least in a case as simple as this one. Scanner class exposes a hasNextInt method for this very purpose. I agree with bracing, I wouldn't skip braces. \$\endgroup\$ Oct 23, 2015 at 13:16
  • 1
    \$\begingroup\$ Oh, I didn't even know about that... let me edit that in, thanks for pointing it out \$\endgroup\$
    – Phrancis
    Oct 23, 2015 at 13:20
4
\$\begingroup\$

Three cases are possible: start < end, start > end, and start == end. You have only covered the first two. I would expect that if start == end, then it prints one number.

It would be nice to have one generic solution that handles all three cases without copy-and-pasted code.

Ending the output with a space rather than a newline is untidy. Even if the challenge doesn't specify exactly how the output should be terminated, I would put a newline there as a matter of convention. In fact, since the challenge doesn't specify a delimiter, I'd just use System.out.println(i) for all of the numbers.

The way you omitted the "optional" braces is really nasty. Don't do that! When you omit braces like that, someday you will be blamed for introducing a bug, perhaps a costly one.

My suggested solution:

public static void main(String[] args) {
    try (Scanner scan = new Scanner(System.in)) {
        int start = scan.nextInt();
        int end = scan.nextInt();

        for (int delta = (start < end) ? +1 : -1; ; start += delta) {
            System.out.println(start);
            if (start == end) break;
        }
    }
}
\$\endgroup\$
4
\$\begingroup\$

I don't know the criteria this online judge uses, but these loops are repetitive.

if(start < end)
    for(int i = start; i <= end; i++)
        System.out.print(i + " ");

else if(start > end)
    for(int i = start; i >= end; i--)
        System.out.print(i + " ");  

else is redundant here, by the way - start couldn't be at the same time smaller and greater than end.

It could be replaced by:

int step = start < end ? 1 : -1;
for (int i = start; i != end + step; i += step) {
    System.out.print(i + " ");
}

Also note that your original code ignored possibility that start == end (it wouldn't print anything in that case), whereas the task description doesn't rule it out.

\$\endgroup\$
4
  • \$\begingroup\$ start isn't smaller and greater than end at the same time. I've put an if statement. If start > end, enter the loop, or else enter the next loop. \$\endgroup\$
    – Kj45
    Oct 23, 2015 at 13:05
  • \$\begingroup\$ @HovigArtinian and for this very reason your else is redundant. These two conditions are mutually exclusive already. It's not like removing else could cause both loops to execute. So basically it's a safeguard against the impossible (unless start or end were modified within the first if block, which isn't the case). \$\endgroup\$ Oct 23, 2015 at 13:12
  • 1
    \$\begingroup\$ @KonradMorawski, Isn't your min/max introducing a bug? If the user inputs start > end, the step is correctly set to -1, but from is then set to end, so in effect you'll have a never ending loop... \$\endgroup\$
    – holroy
    Oct 23, 2015 at 19:28
  • \$\begingroup\$ @holroy hm, I think you're right. I overdid it :) I'll update the code. Good catch, thanks \$\endgroup\$ Oct 25, 2015 at 16:06
2
\$\begingroup\$

Here are some issues in your code as I see it:

  • Repetitive for-loop – You can use one for-loop, if you just switch the step of it
  • No input validation – You don't verify that your input is integer, or loop if it fails
  • No new line at end of output – This is a minor, but you don't end the output with a new line. And it could be argued that you possibly should try to insert newlines somewhere along the way
  • No documentation or comments – It is always nice to have a little documentation, at least if the code isn't dead obivous
  • Missing braces around if- or for-blocks – It is better to always include the braces, to keep you safe of indentation errors and unclear code

If we in addition add a little string.formatting, a static initialization of the scanner, and add a function for validating integer input, we can get the following code:

import java.util.Scanner;

class Main {
    static Scanner scanner;

    /** Make a scanner available for scanInt */
    static {
        scanner = new Scanner(System.in);
    }

    /** Read two integers, and print counting from start through end */
    public static void main(String[] args) {

        int start = scanInt("Enter start number: ");
        int end = scanInt("Enter end number: ");

        int step = start < end ? 1 : -1;

        // Correct end, in order to use inequality test in for-loop
        end += step;

        for (int i = start; i != end; i += step) {
            System.out.print(String.format("%d ", i));
        }
        System.out.println();
    }

    /** Generic function guaranteed to return an int */
    static public int scanInt(String prompt) {

        System.out.print(prompt);
        while (!scanner.hasNextInt()) {
            System.out.print("Input must be a whole number. Try again: ");
            scanner.next();
        }
        return scanner.nextInt();
    }
}
\$\endgroup\$
2
\$\begingroup\$

If Java 8 is applicable here, IntStream.rangeClosed() will be a good candidate for generating the number sequence. However, since it only generates in an increasing order, you'll need to apply some useful arithmetic to reverse the values:

private static String getSequence(int a, int b) {
    return IntStream.rangeClosed(Math.min(a, b), Math.max(a, b))
                    .map(i -> b < a ? b - i + a : i)
                    .mapToObj(String::valueOf).collect(Collectors.joining(" "));
}

The first map() uses b - i + a for each element of the stream if b < a, i.e. for the decreasing order. The second map() simply generates the desired space-delimited String result.

With the help of try-with-resources and a method to handle validation from the Scanner instance, the Java 8 implementation can be as such:

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) {
        System.out.println(getSequence(getInteger(scanner), getInteger(scanner)));
    }
}

private static int getInteger(Scanner scanner) {
    while (!scanner.hasNextInt()) {
        System.out.println("Please enter integers only.");
        scanner.next();
    }
    return scanner.nextInt();
}

private static String getSequence(int a, int b) {
    return IntStream.rangeClosed(Math.min(a, b), Math.max(a, b))
                    .map(i -> b < a ? b - i + a : i)
                    .mapToObj(String::valueOf).collect(Collectors.joining(" "));
}

One thing to note, the use of scanner.next() within the while-loop is to 'reset' the Scanner token parsing, when it encounters an invalid input.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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