Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I posted this an answer this to a question about drawing a random line from a file too large to put into memory. I hacked the below code together. In essence, this is what Reservoir Sampling does, in pseudo-code:

Scan over the 'tape'
    Put the first 'n' samples in a reservoir(of size n)
    // samples are lines of a document, numbers, whatever
    After the first 'n' :
        Pick a random number between 1 and NumberOfLinesCounted
        if the number is between 1 and NumberOfLinesCounted
            replace an existing line with a 1/n chance

Here is the code designed to scan over a document, with said sampling method:

import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.List;

public class reservoirSampling {

    public static void main(String[] args) throws FileNotFoundException, IOException{
        Sampler mySampler = new Sampler();
        List<String> myList = mySampler.sampler(10);
        for(int index = 0;index<myList.size();index++){
            System.out.println(myList.get(index));
        }
    }
}

import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.Scanner;

public class Sampler {

    public Sampler(){}
    public List<String> sampler (int reservoirSize) throws FileNotFoundException, IOException
    {
        String currentLine=null;
        //reservoirList is where our selected lines stored
        List <String> reservoirList= new ArrayList<String>(reservoirSize); 
        // we will use this counter to count the current line number while iterating
        int count=0; 

        Random ra = new Random();
        int randomNumber = 0;
        Scanner sc = new Scanner(new File("Open_source.html")).useDelimiter("\n");
        while (sc.hasNext())
        {
            currentLine = sc.next();
            count ++;
            if (count<=reservoirSize)
            {
                reservoirList.add(currentLine);
            }
            else if ((randomNumber = (int) ra.nextInt(count))<reservoirSize)
            {
                reservoirList.set(randomNumber, currentLine);
            }
        }
        return reservoirList;
    }
}

I'm using something very similar on a project I am on at the moment (i.e. the code is close enough I can transfer any changes easily by hand).

Is this a) efficient, and b) actually reservoir sampling (with equal odds of any line being drawn)?

share|improve this question

1 Answer

At the first glance

You can use Scanner.netxLine() available on all OS.

(randomNumber = (int) ra.nextInt(count))<reservoirSize 

EDIT suppressed unnecessary lines after comments

share|improve this answer
So use sc.nextLine() not Scanner(...).useDelimeter("\n")? Got it. Also, I don't follow how an else if would miss some lines? Reservoir sampling takes the first n lines then randomly swaps them out. Sometimes it swaps, sometimes it doesn't I thnk I see what you're doing, but that's not quite the point of reservoir sampling I don't think. – Pureferret Oct 4 '12 at 7:46
@Pureferret else if whithout a final else block do not catch all conditions. But it is that you want. So remove while(1) .. line and break line with a } so you remove the negative int randomised and avoid a Exception (no negative integer in an array position) – cl-r Oct 4 '12 at 7:54
Well the only thing not inside both the else and the if statement is the while and the random number, which the iff relies on - why not make them the same line? Also, I should never get a random number less than 0.... – Pureferret Oct 4 '12 at 8:02
@Pureferret java.util.Random return also negative number so you cannot test the two limits in the if( ..) statement . Maodify with EDIT – cl-r Oct 4 '12 at 8:30
From the docsnextI‌​nt(int n): Returns a pseudorandom, uniformly distributed int value between 0 (inclusive) and the specified value (exclusive), drawn from this random number generator's sequence. So no. It can't return a negative. – Pureferret Oct 4 '12 at 8:48
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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