Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

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 exercisecommented 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-topicoff-topic for this site, as it would violate the "Do I want the code to be good code" rule.

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.

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.

Source Link
200_success
  • 144.2k
  • 22
  • 188
  • 473

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.