0
\$\begingroup\$

The question given to me was as follows : WAP to take the input from the user a path and a word. If the path is a text file, search the word in the file and print the number of occurrences. If the path is a folder, read the files in the folder and print the number of occurrences in all the text files of the word.

Is what I did an effecient way of going about it? Please ignore indentations and lack of comments.

This was my code :

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

public class TestOccurances {

public static void main(String[] args) {
Scanner sc1 = new Scanner(System.in);
String path, word;
System.out.println("Enter a path");
path = sc1.nextLine();
System.out.println("Enter the word");
word = sc1.nextLine();
sc1.close();
File f = new File(path);
BufferedReader br = null;
List<String> list = new ArrayList<String>();
try {
//Checking to see if its a .txt file
if(f.exists() && f.isFile() && f.getName().endsWith(".txt")){
    br  = new BufferedReader(new FileReader(path));
    String line;
    while((line=br.readLine())!=null) {
        String[] arr = line.split(" ");
        for(String str : arr)
        list.add(str);
        }
    System.out.println("Single text file, Frequency = "+Collections.frequency(list, word));
    }
//Checking to see if its a folder
if(f.exists() && f.isDirectory()) {
    File[] allfiles = f.listFiles();
    List<File>  files= new ArrayList<File>();
    for(File f1 : allfiles) {
        if(f1.getName().endsWith(".txt"))
            files.add(f1);
    }
    //Loop through each text file and do the same as above
    for(File file : files) {
        List<String> occurances = new ArrayList<String>();
        br = new BufferedReader(new FileReader(file.getPath()));
        String line;
        while((line=br.readLine())!=null) {
            String[] arr = line.split(" ");
            for(String str : arr)
            occurances.add(str);
            }
        System.out.println("File path :"+file.getName()+" , Frequency = "+Collections.frequency(occurances, word));
        }
    }
}catch(IOException e) {
    e.printStackTrace();
    }
finally {
    if(br!=null) {
        try {
            br.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        }
    }
}

}

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

While the functional approach may not always be the best solution, I find using Streams on files very convenient.

If you need to extend the second requirement on directories even further to include all .txt files in subdirectories, you can easily recurse through and flatmap all of the subdirectories until you are left with just a single stream of .txt files.

Code:

public static void main(String[] args) throws IOException {

    Scanner inputReader = new Scanner(System.in);

    System.out.println("Enter a path");
    String inputPath = inputReader.nextLine();
    System.out.println("Enter the word");
    String targetWord = inputReader.nextLine();

    File file = new File(inputPath);

    long numOccurances = 0;

    if(file.isFile() && file.getName().endsWith(".txt")){

        numOccurances = toLineStreamFromFile(file)
                            .flatMap(str -> Arrays.stream(str.split("\\s")))
                            .filter(str -> str.equals(targetWord))
                            .count();

    }else if(file.isDirectory()){

        numOccurances = Arrays.stream(file.listFiles(pathname -> pathname.toString().endsWith(".txt")))
                              .flatMap(Main::toLineStreamFromFile)
                              .flatMap(str -> Arrays.stream(str.split("\\s")))
                              .filter(str -> str.equals(targetWord))
                              .count();
    }

    System.out.println(numOccurances);
}

public static Stream<String> toLineStreamFromFile(File file){
    try {
        return Files.lines(file.toPath());
    } catch (IOException e) {
        e.printStackTrace();
    }
    return Stream.empty();
}
\$\endgroup\$
0
\$\begingroup\$

since you requested to focus on effeciency only, I will do just that, although there is a lot to be said about the quality of the code.

first of all, why do you split the line on white space only? what if there is a tab between the words? you should use the predefined class \s which includes all white space characters (incl' tab, new line, and more)

secondly, you do the split into an array, then add the items one-by-one into a list and then search the list using Collections.frequency why not use the loop on the array to compare its items to word and just increase a counter? this saves the loop inside Collections.frequency (for sure there is one) and also does not require to load all words of a file into memory

EDIT following the comment below:

if you need to update the file, there is still no reason to load the words into list. for one, in the list you lose the separation to lines so you will write just one line to the new file? second, why not read one line, process it (either count or make a new modified line) and write the modified line to the output? (modifying a file inline is not recomended!)

regarind code quality, you process a single file separate from processing directory. but processing directory is processing single files in that directory so why not put processing of single file in a separate method and then you can call it for process a single file or in a loop for files in the directory

\$\endgroup\$
2
  • \$\begingroup\$ I added all the items to the list, because I may need to modify the contents of the file. for instance, I may need to replace all occurrences of certain words with another word. The white space part is true, can definetly improve on that as you said. Can you tell more about the quality of code too? I wanna know how I can improve and also the best practices. \$\endgroup\$
    – LoneStar
    Sep 24, 2017 at 13:38
  • \$\begingroup\$ see edited answer \$\endgroup\$ Sep 24, 2017 at 13:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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