Prioritize objects using PriorityQueue and custom comparator in Java

Question

I have a list of objects and a query. I need to rank the objects, based on how do they match the query. An object would match the query 100% if it contains all properties in the query. My program works correctly, but I have a design issue. From the console input, I have a location of a these objects. In this location I need to find all type of specific objects, and to perform the query on them. Based on the result of this query, I need t prioritize them. So from my App.class I create a Map between a object and set of its properties (I can't use the object directly). Then, again from the console input, I read the query. There can be many queries. For all query I create new RankingSystem object (which, I don't like, I don't think I should create new object for every new query), where RankingSystem has two fields - the query itself and the map between Object and its set of properties.

Then, for every query, which I read from the console, I create new RankingSystem object. Again, I don't think this is correct. However, if I put private Set<String> query as a parameter then I have problems with creating a custom comparator. I thought of extracting the comparator in a separate class, yet, I do need to put there the computeRank method. If I do so, then I have to create the comparator with two fields (dictionary and query), because I need them for the computeRank method. Another thing (which stops me from creating the comparator in a separate class), is that I am using computeRank in the findBestMatches method, to print out the percentage of matches. I must print the matching percentage, so I need to have this method (computeRank) in the RankingSystem class. I am using it for my custom comparator as well, so if I extract my custom comparator in a separate class, I will have the same method twice (which is obvious code repetition, which is bad). So I decided to put the comparator inside my RankingSystem class. Is this approach OK, or is there a better approach? Also, is there a way to avoid the creation of an RankingSystem object every time when I have a new query? Here is my RankingSystem class:

 public class RankingSystem {
 private Map<File, Set<String>> dictionary;
 //Q1: is there a way to avoid putting the query as a class field? At the moment it seems as the right solution, because I need it for my custom comparator, but this way I create new object for each query?
 private Set<String> query;
 public RankingSystem(Map<File, Set<String>> dictionary, Set<String> query) {
 this.dictionary = dictionary;
 this.query = query;
 }
 private PriorityQueue<File> compareFiles(){
 PriorityQueue<File> queue = new PriorityQueue<File>(10, fileComparator);
 for(File file: dictionary.keySet()) {
 if(computeFileRank(file) != 0) {
 queue.add(file);
 }
 }
 return queue;
 }
 public void findBestMatches() {
 PriorityQueue<File> queue = compareFiles();
 if(queue.isEmpty()) {
 System.out.println("no matches found");
 }
 int counter = 0;
 while(!queue.isEmpty() && counter < 10) {
 File file = queue.poll();
 System.out.println(file.getName() + ":" + computeFileRank(file) + "%");
 counter++;
 }
 }
 //Q2: is it OK leaving the comparator like this? I tried to extract it into a separate class, but that way I get code repetition (of computeRank method) and I have to pass the dictionary and the query as fields again? 
 private Comparator<File> fileComparator = new Comparator<File>() {
 public int compare(File f1, File f2) {
 if (computeFileRank(f1) > computeFileRank(f2))
 return -1;
 if (computeFileRank(f1) < computeFileRank(f2))
 return 1;
 return 0;
 } 
 };
 private double computeFileRank(File file) {
 int matches = 0;
 int totalCount = query.size();
 Set<String> wordsInFile = dictionary.get(file);
 for(String word: query) {
 if(wordsInFile.contains(word)) {
 matches++;
 }
 }
 return matches*100/totalCount;
 }
}

Just for the sake of trying, I tried to move my custom comparator in a separate class and this is how it looks:

public class FileComparator implements Comparator<File>{
 //I dont like that I need to pass the query and the dictionary again
 private Set<String> query;
 private Map<File, Set<String>> dictionary;
 public FileComparator(Set<String> query, Map<File, Set<String>> dictionary) {
 this.query = query;
 this.dictionary = dictionary;
 }
 @Override
 public int compare(File f1, File f2) {
 if (computeFileRank(f1, query) > computeFileRank(f2, query))
 return -1;
 if (computeFileRank(f1, query) < computeFileRank(f2, query))
 return 1;
 return 0;
 } 
 //I dont like that I have the same method on 2 places
 private double computeFileRank(File file, Set<String> query) {
 int matches = 0;
 int totalCount = query.size();
 Set<String> wordsInFile = dictionary.get(file);
 for(String word: query) {
 if(wordsInFile.contains(word)) {
 matches++;
 }
 }
 return matches*100/totalCount;
 }
}

• dfhwze: Please don't change a question significantly after answers are available. You can always ask a follow-up question instead. I have rollbacked your changes.

Answer 1

You are computing a rank 4 times here. Why not store the results in a variable and compute only 2 times?

public int compare(File f1, File f2) {
 if (computeFileRank(f1) > computeFileRank(f2))
 return -1;
 if (computeFileRank(f1) < computeFileRank(f2))
 return 1;
 return 0;
 } 
};

public int compare(File f1, File f2) {
 final int rank1 = computeFileRank(f1);
 final int rank2 = computeFileRank(f2);
 if (rank1 > rank2)
 return -1;
 if (rank1 < rank2)
 return 1;
 return 0;
 } 
};

---

Can totalCount be 0? Do you want to divide by 0 or have different behavior in such case?

 private double computeFileRank(File file, Set<String> query) {
 // ..
 int totalCount = query.size();
 // ..
 return matches*100/totalCount;
 }