3
\$\begingroup\$

I'm writing a program for my intro to comp sci course where I'm supposed to prompt the user to enter a stream of integers, and then return the largest two. We were given the hint to use a scanner and a while loop with the condition .hasnext.

Please review my code:

package Homework;

import java.util.*;

public class Question1 {

public static void main(String[] args) {
    System.out.println("Please type your integers");
    Scanner kbd = new Scanner(System.in);
    int big = 0;
    int big2 = 0;
    while(kbd.hasNext())
    { 
       int num = kbd.nextInt();
       if(num > big) { // opens If statement
          big2 = big;
          big = num;
        } else if((num < big) && (num > big2))  {
           big2 = num;
        } 
    } 
    System.out.print(big);
    System.out.print(big2);
 } 
}
\$\endgroup\$
3
\$\begingroup\$

Your logic is actually correct. Perhaps you just don't know what to type in your terminal window to terminate the input stream. After typing in your numbers, you need to send an end-of-file ("EOF") character. On Unix, you do that by hitting ControlD. On Windows, it's ControlZ.

Note that System.in is the program's "standard input" stream, which doesn't have to come straight from the keyboard. You can also do

echo 3 1 4 1 5 9 | java Homework.Question1

or

java Homework.Question1 < numbers.txt

Therefore, kbd is a misnomer. Try naming your Scanner more generically, like scanner or input.

You don't want to print your two numbers with no space in between. It will look like one long number!

The placement of your braces is inconsistent, with the ones for the main function and the else if being closest to conventions. The brace opening the while loop should be on the same line as the while keyword. The brace closing the if block should be pulled further left, aligned with the if keyword, and else can come immediately after.

public class Question1 {
    public static void main(String[] args) {
        ...
        while (...) {
            ...
            if (...) {
                ...
            } else if (...) {
                ...
            }
        }
        System.out.println(...);
        System.out.println(...);
    }
}

You have interpreted the exercise such that duplicate entries in the input are ignored. That's the easier way. If the exercise requires you to print the same number twice when there is a tie for the maximum, then the program is trickier to implement.

\$\endgroup\$
  • \$\begingroup\$ I don't know if I'm doing it right or not but echo "3 1 4 1 5 9" | java Homework.Question1 is giving me InputMismatchException but echo 3 1 4 1 5 9 | java Homework.Question1 [no double quotes] is giving me correct output... \$\endgroup\$ – Anirban Nag 'tintinmj' Sep 16 '13 at 13:35
  • 1
    \$\begingroup\$ @tintinmj Oh, you must be on Windows, with different quoting behaviour. I put the double quotes there out of habit on Unix. They aren't necessary, so I'll edit them out of my answer. \$\endgroup\$ – 200_success Sep 17 '13 at 20:58
  • \$\begingroup\$ Yes I'm working in Windows. \$\endgroup\$ – Anirban Nag 'tintinmj' Sep 17 '13 at 21:09
2
\$\begingroup\$
package Homework;

Just for reference, package names are supposed to be lowercase and give information about the origin or author of the package, for example:

package org.yourdomain.yourproject.subpackage;
package com.gmail.youremailaddress.yourproject.subpackage;

It doesn't matter for tests, prototypes and similar, but please keep it in mind.


import java.util.*;

Please don't import whole packages. Only import things you need. A half-decent IDE should be able to manage the imports for you anyway.


Scanner kbd = new Scanner(System.in);

As already said, System.in is not necessarily the keyboard, renaming this variable is advised.


while(kbd.hasNext())
{

Java uses a modified K&R-Style for braces, meaning that opening braces are on the same line.

while (scanner.hasNext()) {

if(num > big) { // opens If statement

Such comments are sign that something is wrong with your code. In this case it seems like a left-over from a previous iteration, but generally if you need to comment on braces and where they belong to, you're doing something wrong.


System.out.print(big);
System.out.print(big2);

Here is a bug, you'll print the output like this:

25984

You should use println() and some informative text:

System.out.println("First: " + Integer.toString(big));
System.out.println("Second: " + Integer.toString(big2));
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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