# Array sort-rank class

I had a task where I was supposed to write a solution to sort entries(Name, score - in an array/list) in order of score and generate ranking based on higher score to lower scores.

I tried doing that, code is working what it was supposed to do but it seems very ambiguous and tightly coupled, I can't even write a unit test for it.

Could someone have a look and suggest.

I have pasted main method class, If anyone wants me to paste other relevent classes as well so do ask me please.

Any suggestion(s)/workaround(s) would be much appreciated.

public class JournalAnalyzer {
final static int LENGTH_OF_JOURNAL = 5; //Length Journal array
static int count = 0;
static Integer ranking = 1;
static Journal[] journals = new Journal[LENGTH_OF_JOURNAL];
static ArrayList<Journal> list = new ArrayList<Journal>();
static Object[] scoreList = new Object[LENGTH_OF_JOURNAL];
static String journalName;
static Double journalScore;
static Integer journalRank;
static Boolean journalReview;

private static void initData(Journal[] journals) {
journals[0] = new Journal(0, "Journal A", 5.6, false);
journals[1] = new Journal(0, "Journal B", 2.6, false);
journals[2] = new Journal(0, "Journal C", 3.2, false);
journals[3] = new Journal(0, "Journal D", 4.1, false);
journals[4] = new Journal(0, "Journal E", 1.6, false);
}

private static int fillJournalList(int count, Journal[] journals,
int LENGTH_OF_JOURNAL, ArrayList<Journal> list) {

for (int i = 0; i < LENGTH_OF_JOURNAL; i++) {
journalRank = new Integer(journals[i].getRank());
journalName = new String(journals[i].getJournal());
journalScore = new Double(journals[i].getScore());
journalReview = new Boolean(journals[i].getIsReviewed());

if (!journalReview) {
Journal value = new Journal(journalRank, journalName,
journalScore, journalReview);
count++;
} else {
continue;
}
}
return count;
}

private static void GenerateRanking(Integer ranking, int count,
ArrayList<Journal> list, Object[] scoreList) {
Double scoreComparatorOne;
Double scoreComparatorTwo;

Comparator<Journal> scores = new SortScore();
Collections.sort(list, scores);

for (int i = 0; i < count; i++) {
scoreComparatorOne = (list.get(i)).getScore();
if (i == 0) {
scoreComparatorTwo = (list.get(i)).getScore();
} else {
scoreComparatorTwo = (list.get(i - 1)).getScore();
}
if (scoreComparatorOne.equals(scoreComparatorTwo)) {
if (ranking == 0) {
ranking += 1;
} else {
ranking = ranking;
}
} else {
ranking = i + 1;
}

(list.get(i)).setRank(ranking);
scoreList[i] = (list.get(i));

}
}

private static void displayJournal(int counter, Object[] scoreList) {
JournalOutput printer = new JournalOutput();
printer.display(count, scoreList);
System.exit(0);
}

public static void main(String[] args) throws IOException {
initData(journals);
count = fillJournalList(count, journals, LENGTH_OF_JOURNAL, list);
GenerateRanking(ranking, count, list, scoreList);
displayJournal(count, scoreList);
}
}


My question is how to minimize and simplify this code so that I can easily write a unit test for this class

## migrated from stackoverflow.comNov 30 '12 at 11:21

This question came from our site for professional and enthusiast programmers.

• What is your question? – Code-Guru Nov 29 '12 at 23:45
• My question is how to minimize and simplify this code so that I can easily write a unit test for this class. – Ali Jay Nov 29 '12 at 23:52

You could change this:

for (int i = 0; i < count; i++) {
scoreComparatorOne = (list.get(i)).getScore();
if (i == 0) {
scoreComparatorTwo = (list.get(i)).getScore();
} else {
scoreComparatorTwo = (list.get(i - 1)).getScore();
}
if (scoreComparatorOne.equals(scoreComparatorTwo)) {
if (ranking == 0) {
ranking += 1;
} else {
ranking = ranking;
}
} else {
ranking = i + 1;
}
(list.get(i)).setRank(ranking);
scoreList[i] = (list.get(i));
}


Theres no need to verify all the time this conditional:

if (i == 0) {
scoreComparatorTwo = (list.get(i)).getScore();
} else {
scoreComparatorTwo = (list.get(i - 1)).getScore();
}


inside the loop. Since i it will be equal to 0 only once.

Modifying to:

...
Double scoreComparatorOne = (list.get(0)).getScore();
Double scoreComparatorTwo = (list.get(0)).getScore();
ranking = getRating(scoreComparatorOne,scoreComparatorTwo,ranking,0);
(list.get(0)).setRank(ranking);
scoreList[0] = (list.get(0));

for (int i = 1; i < count; i++){
scoreComparatorOne = (list.get(i)).getScore();
scoreComparatorTwo = (list.get(i - 1)).getScore();
ranking = getRating(scoreComparatorOne,scoreComparatorTwo,ranking,i);
(list.get(i)).setRank(ranking);
scoreList[i] = (list.get(i));
}


where getRating is:

 public Integer getRating (Double sCOne, Double sCTwo, Integer ranking, int i)
{
if(sCOne.equals(sCTwo))
return (ranking == 0) ? 1 : ranking;
else return i + 1;
}


also you can turn your fillJournalList into (but this is not a major problem):

private static int fillJournalList(Journal[] journals,
int LENGTH_OF_JOURNAL, ArrayList<Journal> list) {

for (int i = 0; i < LENGTH_OF_JOURNAL; i++){

if (!journals[i].getIsReviewed())
new String(journals[i].getJournal(),
new Double(journals[i].getScore(),
false);

return list.size();


}

This way you only create the new Objects when you will insert them. Not in every for iteration. With the above modification there no need to this:

public class JournalAnalyzer {
...
static String journalName;
static Double journalScore;
static Integer journalRank;
static Boolean journalReview;


You can remove all four variables.

• Hi, I made changes to the code, now its throwing NullPointerException. Due to scoreList skipping to fill scorelist[0] element. – Ali Jay Nov 30 '12 at 9:52
• @AliJay did you manage to solve? – dreamcrash Nov 30 '12 at 12:13
• @@dreamcrash I have made changes like mentioned in my last comment, now its throwing NullPointerException. Due to scoreList skipping to fill scorelist[0] element. And it is due to GenerateRanking method where for loop starts from counter 1. – Ali Jay Nov 30 '12 at 12:17
• post your code here codepaste.net and post it back the link here so i can see it – dreamcrash Nov 30 '12 at 12:21
• here you go <codepaste.net/87imtg> Cheers – Ali Jay Nov 30 '12 at 12:26