5
\$\begingroup\$

I have implemented a class which implements a list of my custom DateObj. I have sorted the list in a peculiar manner based on the current month. I achieved my desired results, but I'm required to call Collections.sort() four times to achieve it. I feel this could be improved. Any thoughts?

//Custom Date Object public class DateObj { private int year; private int month; private int date; DateObj(int year, int month ,int date){ this.year = year; this.month = month; this.date = date; } public Integer getYear() {return year;} public Integer getMonth() {return month;} public Integer getDate() {return date;} } 

The custom sort class:

import java.util.ArrayList; import java.util.Collections; import java.util.List; public class CustomListSort { static List<DateObj> list = new ArrayList<>(); public static void main(String a[]){ createRandomDateObjs(); Integer CURRENTMONTH = 6; //Sort Months Descending Collections.sort(list,(arg0,arg1)-> arg1.getMonth().compareTo(arg0.getMonth())); //Sort by keeping the object with nearest largest next month first Collections.sort(list,(arg0,arg1)->CURRENTMONTH.compareTo(arg0.getMonth())); //Sort the months which were pushed back in an ascending order. Collections.sort(list,(arg0,arg1)->(arg0.getMonth()<=CURRENTMONTH && arg1.getMonth()<=CURRENTMONTH)? arg0.getMonth().compareTo(arg1.getMonth()):0); //If months same sort them in ascending manner based on their dates Collections.sort(list, (arg0,arg1)->(arg0.getMonth() == arg1.getMonth()) ? arg0.getDate().compareTo(arg1.getDate()) : 0); System.out.println("\nDESIRED OUTPUT"); System.out.println("\nMM - DD - YYYY \n"); for(int i = 0 ; i<list.size();i++) System.out.println(list.get(i).getMonth() +" - "+list.get(i).getDate()+" - "+list.get(i).getYear()); } static void createRandomDateObjs(){ for(int i = 0 ; i<20 ; i++){ int y = 1980; y+=(int)(Math.random()*55); int m = (int) (Math.random()*11); int d = (int) (Math.random()*30); list.add(new DateObj(++y,++m,++d)); //Pre-increment to change 0's to 1's } } 

Initially I had implemented it using Comparators but the Lambda made the code much cleaner.

Desired output:

MM - DD - YYYY 7 - 20 - 2023 7 - 18 - 2027 8 - 30 - 2010 8 - 12 - 2020 9 - 21 - 2006 10 - 23 - 2008 11 - 19 - 2000 11 - 16 - 2033 11 - 13 - 1989 12 - 13 - 2019 1 - 10 - 1985 1 - 15 - 2016 4 - 10 - 2021 4 - 13 - 2022 4 - 14 - 2004 5 - 1 - 2025 5 - 4 - 2014 5 - 20 - 2023 5 - 21 - 2022 6 - 29 - 1990 
\$\endgroup\$
1
  • 5
    \$\begingroup\$You've been told on your stack overflow question that you can use Comparator.thenComparing on Java 8 to achieve this...\$\endgroup\$
    – Vogel612
    CommentedJan 24, 2016 at 12:24

2 Answers 2

6
\$\begingroup\$

A Comparator is the correct approach, you just need to mod the month by the current month before comparing them. The easiest way to do this is to introduce a static variable currentMonth to hold the current month for all instances of DateObj.

Add DateObj.setCurrentMonth(CURRENTMONTH); to your Main, and use the code below:

//Custom Date Object class DateObj implements Comparable<DateObj>{ private int year; private int month; private int date; private static int currentMonth; DateObj(int year, int month ,int date){ this.year = year; this.month = month; this.date = date; } public Integer getYear() {return year;} public Integer getMonth() {return month;} public Integer getDate() {return date;} public Integer getCurrentMonth() { return DateObj.currentMonth;} public static void setCurrentMonth(int currentMonth){ DateObj.currentMonth = currentMonth; } @Override public int compareTo(DateObj o) { int months = (month+(12-currentMonth))%12 - (o.month+(12-currentMonth))%12; if(months == 0) return this.date-o.date; else{ return months; } } } 

EDIT: An alternative approach is to simply define the Comparator in your main body. This avoids the use a static value to hold currentMonth:

 Collections.sort(list, (o1,o2)->{ int months = (o1.getMonth()+(12-CURRENTMONTH))%12 - (o2.getMonth()+(12-CURRENTMONTH))%12; if(months == 0) return o1.getDate()-o2.getDate(); else{ return months; } }); 

Thanks to Mr.Wiggles for the recommendation.

\$\endgroup\$
    4
    \$\begingroup\$

    Your current code has several flaws:

    • Do not return an Integer object when an int suffice. It might create an unnecessary object. I would rewrite your getter like this

      public int getYear() { return year; } 
    • You should properly indent and format your code, as it makes it a lot easier to read. For example, the getter can be written more clearly like the snippet above.
    • You are indeed comparing 4 times the list when you can do it in one pass.
    • Your usage of static is awkward. You should get rid of it and make the method createRandomDateObjs() return the created list of DateObj.
    • Don't name your local variable CURRENTMONTH. If it is a constant, make it private static final int instead.

    The heart of the comparator is about comparing the month of the dates. Instead of using 3 separate comparator, consider the following one. Given 2 DateObjo1 and o2:

    • If the months of o1 and o2 are both strictly greater than 6, the one being closer to 6 should be sorted before the other one.
    • If the months of o1 and o2 are both lower or equal to 6, the one being closer to 6 should be sorted after the other one.

    (Those two conditions takes care of the fact that the month 6 should be sorted at the end of the list)

    • If they have different signs, we need an extra logic:
      • If the month of o1 is strictly after 6 (so the month of o2 is before 6), it means that o1 should be sorted before o2 so we must return a negative value (like -1).
      • On the contrary, if the month of o1 is before or equal to 6 (so the month of o2 is after 6), it means that o1 should be sorted after o2 so we must return a positive value (like 1).

    This would an implementation of this comparator using a lambda expression:

     Comparator<DateObj> comparator = (o1, o2) -> { int diff1 = o1.getMonth() - CURRENTMONTH; int diff2 = o2.getMonth() - CURRENTMONTH; if (diff1 > 0 && diff2 > 0 || diff1 <= 0 && diff2 <= 0) { return diff1 - diff2; } else if (diff1 > 0) { return -1; } else if (diff1 <= 0) { return 1; } return 0; }; 

    The advantage with this reasoning over using modular arithmetic is that it will work for whatever value (even negative value). The answer by Austin D assumes that all value are positive and between 0 and 11 (it could be made mode flexible but then you would first have to determine the maximum value). It only works for this particular case (because a month always verifies that assumption).

    Then, in case the month are equal, your code is sorting based on the date field. With Java 8, you can compose multiple comparator with Comparator.thenComparing. In this case, we will then compare with an int so we can use Comparator.thenComparingInt(keyExtractor), where the key extractor returns the date.

    The final comparator would be:

     Comparator<DateObj> comparator = (o1, o2) -> { int diff1 = o1.getMonth() - CURRENTMONTH; int diff2 = o2.getMonth() - CURRENTMONTH; if (diff1 > 0 && diff2 > 0 || diff1 <= 0 && diff2 <= 0) { return diff1 - diff2; } else if (diff1 > 0) { return -1; } else if (diff1 <= 0) { return 1; } return 0; }; comparator = comparator.thenComparingInt(DateObj::getDate); list.sort(comparator); 
    \$\endgroup\$

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.