Skip to main content
Code Review

Return to Revisions

5 of 5
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

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.
JohnMark13
  • 771
  • 4
  • 11
default

AltStyle によって変換されたページ (->オリジナル) /