cuál es incorrecto con este código?

Question

Estoy leyendo el contenido de los archivos de un directorio. Tengo que separar los archivos de acuerdo a sus nombres y luego leer su contenido. Cuando ejecuto el código simplemente sin necesidad de leer el contenido, todos los archivos se están listadas de un nombre de archivo en particular, pero cuando trato de leer el contenido, lee los contenidos de solamente unos pocos archivos, de hecho, sólo 10 de ellos. Sin embargo, el directorio tiene alrededor de 1000 archivos de un nombre en particular. Estoy publicar el código aquí.

for (i = 0; i <= filenames.length; i++) {
    read = new FileReader("trainfiles/"+filenames[i]);          
    br = new BufferedReader(read);

    if (filenames[i].matches(".*ham.*")) {
        System.out.println("ham:" + filenames[i]);
        while ((lines = br.readLine()) != null) {
            st = new StringTokenizer(lines);
            while (st.hasMoreTokens()) {
                System.out.println(st.nextToken());
            }
        }
        br.close();
    }
}

¿Alguien podría decirme dónde estoy haciendo mal !?
gracias

editar # 1 Hice algunas modificaciones que me han dicho aquí, pero el problema aún persiste, aquí está el código.

for(i=0;i<=filenames.length;i++){
            read = new FileReader("trainfiles/"+filenames[i]);

            br = new BufferedReader(read);

            if(filenames[i].matches(".*ham.*")){
                System.out.println("ham:"+filenames[i]);

                        while((lines = br.readLine())!= null){
                            st = new StringTokenizer(lines);
                            while(st.hasMoreTokens()){
                                System.out.println(st.nextToken());
                            }

                        }

            }
            br.close();
            read.close();




                        }

editar # 2 Ahora las miradas código como este, pero de nuevo ... no me da el resultado que quiero.

for (i = 0; i < filenames.length; i++) {
               try {


                if (filenames[i].matches(".*ham.*")) {
                     read = new FileReader("trainfiles/"+filenames[i]);          
                        br = new BufferedReader(read);
                    System.out.println("ham:" + filenames[i]);
                    while ((lines = br.readLine()) != null) {
                        st = new StringTokenizer(lines);
                        while (st.hasMoreTokens()) {
                            System.out.println(st.nextToken());
                        }
                    }
                }
               } finally {

                read.close();
                br.close();
               }
            }

Answer 1

I would re-write your code like this, and see what output you get:

for (filename : filenames) {
   if (filename.matches(".*ham.*")) {
      System.out.println("ham:" + filename);

      // reset these to null (where are they declared?)
      read = null;   
      br = null;   
      try {
         read = new FileReader("trainfiles/"+filename);          
         br = new BufferedReader(read);

         while ((lines = br.readLine()) != null) {
            System.out.println(lines);
            // st = new StringTokenizer(lines);
            // while (st.hasMoreTokens()) {
            //    System.out.println(st.nextToken());
            // }
         }
      } catch (Exception e) {
         e.printStackTrace();
      } finally {
         if (br != null) br.close();
         if (read != null) read.close();
      }
   } 
}

Some general comments on your original code:

1. Only use a for loop if you actually need the array index. Prefer a for-each loop (i.e. for (filename : filenames) ...).

2. Declare variables in the narrowest scope possible. In this case, you should declare your read and br variables where I initialize them to null.

3. Never open a file unless you're going to use it. Here, that means opening it inside the conditional block.

4. Since opening a file can throw an exception, br may not get initialized, in which case you can't close it. You need to check for null first.

Answer 2

First of all you should use i<filenames.length. Second, matches expects a regular expression, not *-globs. The expression you used is a valid regular expression for [something]ham[something] - is that what you meant?

I don't think you need to close the Filereader - I think BR's close propagates up. But that's worth checking. EDIT as was mentioned, you need to always close the file, outside the if.

Answer 3

You should close your FileReader object read as well.

Unless this is homework, I would also suggest you take a look at commons-io.

EDIT #1: I would suggest doing both close operations in a finally block.

EDIT #2: Did you try this?

for (i = 0; i <= filenames.length; i++) {
   try {
    read = new FileReader("trainfiles/"+filenames[i]);          
    br = new BufferedReader(read);

    if (filenames[i].matches(".*ham.*")) {
        System.out.println("ham:" + filenames[i]);
        while ((lines = br.readLine()) != null) {
            st = new StringTokenizer(lines);
            while (st.hasMoreTokens()) {
                System.out.println(st.nextToken());
            }
        }
    }
   } finally {
    br.close();
    read.close();
   }
}

Answer 4

1000+ files are a lot of files to read. If it can't read a file it should throw an exception (IOException to be specific). Maybe print the exception message in the catch block and paste it here.

I don't know the StringTokenizer class but does the code give errors when you just print the line without the StringTokenizer?

An other option is to use threads. You have the array of files and then you start some threads who reads a file (producer/consumer problem).

By the way, you can filter files with the class FileFilter.

http://download.oracle.com/javase/1.4.2/docs/api/java/io/File.html#listFiles%28java.io.FileFilter%29