Exceptions to control data read flow

Question

I previously posted a Reservoir-Sampling program, which was basically a test version of this one. This is an assignment. In the code below, I use RunTimeException to control when scanner finishes reading data. Reasons for this being:

1. I cannot use any other library than java.lang.
2. I cannot use Scanner, either. We have been provided a StdIn class which has a static scanner.next() method only for reading strings.

Note: All the code is inside main() because that is how the API demands it.

Can someone review my code, and possibly tell if there's some way I can improve this?

import java.io.BufferedInputStream;
import java.util.Scanner;

public class Subset {

    public static void main(String[] args) {
        int k = Integer.parseInt(args[0]);
        String[] arr = new String[k];
        int i = 0;
        String str;     
        while((str=stdin.readString())!=null && i < arr.length){
            arr[i++] = str;
            // System.out.println(str);
        }
        try{
            for(; (str=stdin.readString())!=null; i++){
                int r = (int)(Math.random()*(i + 1));
                if(r < k){
                    arr[r] = str;
                    //System.out.println(str);
                }
            }
        }catch(RuntimeException e){ // do nothing }     
        for(String s : arr){
            System.out.print(s + " ");
        }
        System.out.println("\n");
    }
}
class stdin{
    private stdin(){}
    private static Scanner sc = new Scanner(new BufferedInputStream(System.in));
    public static String readString(){
        return sc.next();
    }
}

Command line input: echo A B C D E F G H I J | java Subset 3
Output: A F C

To clear out confusion, I am posting what the API and the instructor demands:

Subset client. Write a client program Subset.java that takes a command-line integer k, 
reads in a sequence of N strings from standard input using StdIn.readString(), 
and prints out exactly k of them, uniformly at random. Each item from the sequence can be printed out at most once. 
You may assume that k = 0 and no greater than the number of string on standard input.

% echo A B C D E F G H I | java Subset 3       % echo AA BB BB BB BB BB CC CC | java Subset 8
C                                              BB
G                                              AA
A                                              BB
                                               CC
% echo A B C D E F G H I | java Subset 3       BB
E                                              BB
F                                              CC
G                                              BB
Your client should use only constant space plus one object either of type Deque or of type RandomizedQueue;
use generics properly to avoid casting and compiler warnings. It should also use time and space proportional to 
at most N in the worst case, where N is the number of strings on standard input. 
(For an extra challenge, use space proportional to k.) It should have the following API.

public class Subset {
   public static void main(String[] args)
}

Answer 1

Finished in the comments, can you post the part of the assignment question pertinent to your limited library usage and main method only code? Anyway, if you have been told that a RTE will signify the end of the reading (rather than the null) then this seems marginally better as it makes it clear why you are doing what you are doing:

while (true) {
    try {
        str = stdin.readString();
        if (str == null) {
            break;
        }
        int r = (int)(Math.random()*(i++ + 1));
        if(r < k){
            arr[r] = str;
        }
    }
    catch(RuntimeException rte) {
        //Log the crazy break condition
        break;
    }
} 

It is always a discussion point whether while(true) is acceptable and is normally held up against using a running loop variable. In this case you can intialise a boolean to true and use it as your while condition - setting it to false where the breaks are. It involves extra checks, and makes for harder to read code in my eyes, but hey!

Now, give your variables meaningful names.

EDIT: Post clarification in the comments

So importantly you do not know the length of the input but you want to randomly sample it, and a RumtimeException is unexpected and can make your code blow up? In which case why anymore complex than this?:

public class Subset {

    public static void main(String[] args) {
        int sampleSize = Integer.parseInt(args[0]);
        String[] samples = new String[sampleSize];
        int current = 0;
        String str; 

        //make sure you have at least sampleSize number of samples
        while((str=stdin.readString())!=null && current < sampleSize){
            samples[current++] = str;
        }

        //this will read until the end of the input
        while((str=stdin.readString())!=null) {
            int randomIndex = (int)(Math.random()*(current++));
            if(randomIndex < sampleSize){
                samples[randomIndex] = str;
            }
        }

        for(String sample : samples){
            System.out.print(sample + " ");
        }
        System.out.println("\n");
    }
}

EDIT post comments In your original question on Reservoir Sampling it was important to add +1 to you possible random number as you wanted to pick a value from an array of known size without excluding the final element . However in this version you are using the random number to dictate (with decreasing probability of a hit) where to write the next value read from Stdin into your array of samples.

If you know that the Stdin implementation and that it will through an Exception and that is the only thing you have been told or advised will signify an end to the data (no special characters, no nulls, no maximum length, etc) then the only thing that you can do is catch and handle it and that is perfectly acceptable - in this instance it is not really an Exception condition, it is the norm! Maybe he exercise is in working with bad APIs (which is very common)!

In the latest (very different) version of your code you have this:

for(; ; i++){ 
    int randomIndex = (int)(Math.random()*(i + 1));
    if(randomIndex < sampleSize){
        sample[randomIndex] = StdIn.readString();
    }
 }

You should replace this with code more like my first example where the exception state is handled within the loop. So:

• Replace for loop with while loop
• Because you are using the randomIndex to determine whether to read a value at all into the array you are no longer random sampling, you are just random inserting. Every value read from the pool will get written to the array. It is also important that this could run for a long time or more likely run to an error state when i > MAX_INT, which would be possible with a large dataset and a small sample.

Answer 2

Before we deal with style, let's consider the substance. I don't think that the following code leads to unbiased sampling as you intend:

for(; ; i++){ 
    int randomIndex = (int)(Math.random()*(i + 1));
    if(randomIndex < sampleSize){
        sample[randomIndex] = StdIn.readString();
    }
}

That raises the issue, are you not able to reuse the classes already developed from your previous exercise? In software development, it would be much preferable to call existing code than to reimplement an algorithm using copy-and-paste. Had you done so, this sampling error would have been avoided completely.

Next, I question your statement

All the code is inside the main() because that is how the api demands it.

As I commented in the previous exercise, the API demands that your main() take a single string parameter that will be parsed as an integer representing the number of samples to take, out of the pool of data that will be read from standard input. The API does not demand that all of your code live inside main(). (Your instructor might demand it, but that would just be silly, in my opinion.)

To address your specific question of flow control, catching RuntimeException is a horrible idea. It's much too unspecific; who knows what kind of RuntimeException actually triggered the premature termination of your loop? Furthermore, it's unclear to anyone looking at the code that it is the intended behaviour; exceptions (especially RuntimeExceptions) usually signify for unexpected events. Let's face it, you're catching RuntimeException instead of NoSuchElementException to work around the silly artificial constraint of your exercise that you must not use anything outside java.lang.*.

The correct way to detect the end of input from java.util.Scanner is to call Scanner.hasNext(), and the next best way is to catch NoSuchElementException. If both of those options are artificially closed off from you, then your question is dangerously close to being off-topic for this site, as it would violate the "Do I want the code to be good code" rule.