CODEWITHSUNDEEP
javastaticcomparator
big
big

Reputation: 1938

Non static comparator inner class

I would like to know is it ok to make non static inner class which implements comparator interface.

eg.

public class A {
   private Map<String, Integer> scoreMap;

   private final class myComparator implements Comparator<MyOtherClass> {

       private int calScore(MyOtherClass ob) {
           int score = ob.someValue * ob.otherValue;
           scoreMap.put(ob.getName(), score);
       }

       public int compare(MyOtherClass ob1, MyOtherCLass ob2) {
           return Integer.compare(calScore(ob1), calScore(ob2));
       }   

   }

}

I would like to use comparator class non static because I want to use non static field "num" and want to modify its value. Is it fine to have non static comparator inner class?

Additional Info

For each object I am calculating score inside compare and sorting accordingly. I need to save those scores in a map which I calculated inside comparator and want to use this map in outer class for further calculation.

Upvotes: 0

Views: 1696

Answers (1)

Joeblade
Joeblade

Reputation: 1743

Technically nothing is stopping you. Morally however...

As long as you maintain the idempotency (run multiple times and get same result) then it is not incorrect. However it is a sideeffect. (use this comparator, and some class's values change. run another comparator and nothing changes). Side effects aren't necessarily bad but they make a program more complex and harder to understand and debug.

Try seeing it through the eyes of some poor soul that has to maintain your code. Some value changes and they have no idea why. all they did was compare an unaffiliated list.

anyways your example is very abstract so it's hard to judge the context but usually there is no need to do what you want to do. It is generally something to be avoided unless you have a really good reason for it. And that really good reason shouldn't be "because I don't want to loop over the dataset again" in my opinion.

from your edit:

You are trying to save yourself work by not having to do the recalculating again from the sounds of it. You're saving yourself only a small amount of effort really. why not first calculate the scores, store the result, then sort the results (on score field)

like:

public static class ScoredEntry {
    private SomeGameData data; //or something
    private int score;
    // constructor takes data + score,  imagine getters, setters, I am lazy ok.
}

public List<ScoredEntry> scoreEntries(List<SomeGameData> gameData) {
    List<ScoredEntry> result = new ArrayList<ScoredEntry>();
    for (SomeGameData data : gameData) {
        int score = calculateScore(data);
        result.add(new SCoredEntry(score, data);
    }
    return result;
}

public void sortBySCore(List<ScoredEntry> list) {
    Collections.sort(list, new Comparator<ScoredEntry>() {
        public int compare(SCoredEntry a, ScoredEntry b) {
            // etcetera.
        }
    }
}

Upvotes: 1

Related Questions