Reading data from RandomAccessFile producing incorrect results - Java

Question

I have a text file with 42 lines. Each line has more than 22,000 numbers separated by comma.

I wanted to extract certain numbers from each line, and I have an int array with a length of 1000 containing a 1,000 numbers that I need from each of those 42 lines.

For example if the array contains 43, 1 , 3244, it means that I want the 43th number, the 1st number and the 3244th numbers from each line, starting from the first line ending with the 42nd line.

My for loop does not seem to work, it reads only the first line from the text file that has the 42 lines of 220000 numbers, and I dont know where it goes wrong.

for(int i=0;i<42;i++){ //irretates through the 42 lines of 
    counter=1; // to keep track about on which line the code is working
    System.out.println("Starting line"+i);

    st2=new StringTokenizer(raf1.readLine(),",");
    //raf3 is a RandomAccessFile object containing the 42 lines

    a:while(st2.hasMoreTokens()){
        b=is_there(array2,counter);
        // is_there is a method that compares the rank of the taken being read with 
       //the whole array that has the 1000 numbers that i want. 
        if(b==false){ 
            // if the rank or order of token [e.g. 1st, 432th] we are stopping at 
           //is not among the 1000 numbers in the array 
            counter++;                  
            continue a;
        }
        else{ //if true
            s2=st2.nextToken();
            raf2.writeBytes(s2); //write that token on a new empty text file 
            raf2.writeBytes(","); // follow the number with a comma
            counter++;
        }
    } // end of for loop



public static boolean is_there(int[] x,int y){
    boolean b=false;
    for(int i=0;i<x.length;i++){
        if(x[i]==y) {b=true; break;}
    }
    return b;

Answer 1

One problem you have is that when you find an index that's not in your array, you don't actually skip the token:

if ( b == false ) {
    // don't actually skip the token !!
    counter++;                  
    continue a;
} else {
    s2 = st2.nextToken();
    raf2.writeBytes(s2);
    raf2.writeBytes(",");
    counter++;
}

This means your StringTokenizer gets 1 token behind every time you try to skip.

This could possibly result in an infinite loop for example.

if ( b == false ) {
    // so skip the token !!
    st2.nextToken();

    counter++;                  
    continue a;
} else {
    s2 = st2.nextToken();
    raf2.writeBytes(s2);
    raf2.writeBytes(",");
    counter++;
}

As a side note, the loop can be rewritten more elegantly as follows:

while (st2.hasMoreTokens()) {
    s2 = st2.nextToken();

    if (is_there(array2, counter)) {
        raf2.writeBytes(s2);
        raf2.writeBytes(",");
    }

    ++counter;
}

You should also:

• use more descriptive names for things.
• declare variables in the scope they are used.

Answer 2

The Radiodef answer is correct, however I think there is still one piece missing. The code finds proper numbers, but prints them in one line, because there is no 'next Line' statement after the loop which go through particular line (at least not in the code above), for example like this:

        for(int i=0;i<42;i++){
        counter=1; // to keep track about on which TOKEN the code is working
        System.out.println("Starting line"+i);
        st2=new StringTokenizer(raf1.readLine(),",");
            while(st2.hasMoreTokens()){
                boolean b = is_there(array2,counter);
                if(!b){
                    st2.nextToken();
                }else{
                    String s2=st2.nextToken();
                    raf2.writeBytes(s2 + ",");
                }
                counter++;
            }
            raf2.writeBytes("\r\n");         //next line!
        }

This way, it should read, search and print numbers correctly.

What's more, there is a mistake in comments: counter=1; // to keep track about on which line the code is working. The counter keeps track on which token the loop is working on, not line.

BTW. the is_there method also could take a shorter form:

public static boolean is_there(int[] x,int y){
    for(int i : x){
        if (i == y) return true;
    }
    return false;
}

However, I am not sure, is it more readable.