3
\$\begingroup\$

Assume filteredEvents from below to be a List<Filter>. Basically a list of objects, each of these holding a list of days in the year, among other not-important-at-the-moment data and a specific colour to draw on the calendar, to represent that there are events for that particular colour distinguishable department.

It's used in the context of filtering a Calendar based on the type of events, "Dept1" is one Filter, "Dept2" is another Filter, etc. Each Filter holds all the dates that should appear in the Calendar for that Department.

The Filter and Event classes are at the bottom, for clarification purposes.

Inside the PopulateCalendarWithEvents method I have the following code. What I want to achieve is go through the filterEvents array of Days and sort those based on how many times a given date appears throughout all the Filters.

For instance, if Dept2 and Dept5 both have an event for today 16/04/2018, it should put that date into the twoEventDays only, and put Dept2 and Dept5's colours correspondingly.

public void PopulateCalendarWithEvents ( List<Filter> filteredEvents) {
   //Initiate all the arrays to receive the sorted events. 5 as the maximum number of dots per days is set to 5  
    Collection<CalendarDay> oneEventDays =  new ArrayList<>();
    int[] oneColors = new int[1];
    Collection<CalendarDay> twoEventDays= new ArrayList<>();
    int[] twoColors= new int[2];
    Collection<CalendarDay> threeEventDays= new ArrayList<>();
    int[] threeColors= new int[3];
    Collection<CalendarDay> fourEventDays= new ArrayList<>();
    int[] fourColors= new int[4];
    Collection<CalendarDay> fiveEventDays= new ArrayList<>();
    int[] fiveColors= new int[5];

    //Loop trough all the Filters, and get their Day Arrays
    for(int i=1;i<filteredEvents.size();i++) {
        ArrayList<CalendarDay> currentDays = filteredEvents.get(i).calDayArr;
        for (int x = 0; x < currentDays.size(); x++) {
            if (oneEventDays.contains(currentDays.get(x))) { //If the currently looped date exists in the first array, then this is it's second or more appearance, 
                // and should be moved to a higher number array 
                if (twoEventDays.contains(currentDays.get(x))) {//If the currently looped date exists in the 2nd array, it needs to go into the next one
                    if (threeEventDays.contains((currentDays.get(x)))) {
                        if (fourEventDays.contains((currentDays.get(x)))) {
                            if (fiveEventDays.contains((currentDays.get(x)))) {
                                //As there is room for only so many dots It doesn't matter if I don't add the date and colour here. 
                                // As once you click the date on the Calendar it will show you all events even if they are 100
                            } else {
                                //If the date existed in the 4th List and not the 5th it then goes into the 5th list, including it's colour
                                fiveEventDays.add(currentDays.get(x));
                                System.arraycopy(fourColors, 0, fiveColors, 0, fourColors.length);
                                fiveColors[4] = filteredEvents.get(i).color;
                            }
                        } else {
                            //If the date existed in the 3th List and not the 4th it then goes into the 4th list, including it's colour
                            fourEventDays.add(currentDays.get(x));
                            System.arraycopy(threeColors, 0, fourColors, 0, threeColors.length);
                            fourColors[3] = filteredEvents.get(i).color;
                        }
                    } else {
                        threeEventDays.add(currentDays.get(x));
                        System.arraycopy(twoColors, 0, threeColors, 0, twoColors.length);
                        threeColors[2] = filteredEvents.get(i).color;
                    }
                } else {
                    twoEventDays.add(currentDays.get(x));
                    System.arraycopy(oneColors, 0, twoColors, 0, oneColors.length);
                    twoColors[1] = filteredEvents.get(i).color;
                }
            } else { //If the date doesn't exist anywhere, add it into the first List
                oneEventDays.add(currentDays.get(x));
                oneColors[0] = filteredEvents.get(i).color;
            }
        }
    }

    //Remove the higher number of Dates from the lower lists, so that you don't end up with repeating dates for all Lists up to the maximum one, a date should be into
    oneEventDays.removeAll(twoEventDays);
    twoEventDays.removeAll(threeEventDays);
    threeEventDays.removeAll(fourEventDays);
    fourEventDays.removeAll(fiveEventDays);

    //Send all the Lists and their colors to the Decorator function, that is from the library, loops trough the dates and paints the appropriate number of coloured dots onto each day
    calendarView.addDecorator(new EventDecorator(oneEventDays,oneColors));
    calendarView.addDecorator(new EventDecorator(twoEventDays,twoColors));
    calendarView.addDecorator(new EventDecorator(threeEventDays,threeColors));
    calendarView.addDecorator(new EventDecorator(fourEventDays,fourColors));
    calendarView.addDecorator(new EventDecorator(fiveEventDays,fiveColors));
}

This is how the Filter and Event classes look, though I'm happy enough with those, I've added them just for clarification.

public class Filter {
    public int filterIndex;
    public String filterName;
    public boolean selected;
    public ArrayList<Event> eventList;
    public int color;
    public ArrayList<CalendarDay> calDayArr;

    public Filter(int filterIndex, String filterName, boolean selected, ArrayList<Event> eventList, int color) {
        this.filterIndex = filterIndex;
        this.filterName = filterName;
        this.selected=selected;
        this.eventList=eventList;
        this.color = color;
        if(eventList!=null)
        calDayArr = processCalDays(eventList);
    }

    private ArrayList<CalendarDay> processCalDays(ArrayList<Event> eventList) {
        ArrayList<CalendarDay> calDayArr=new ArrayList<>();

        for(int i=0;i<eventList.size();i++)
        {
            Calendar calendar = Calendar.getInstance();
            calendar.set(Calendar.DAY_OF_MONTH, eventList.get(i).day);
            calendar.set(Calendar.MONTH,eventList.get(i).month);
            calendar.set(Calendar.YEAR,eventList.get(i).year);
            calDayArr.add(CalendarDay.from(calendar));
        }
        return calDayArr;
    }
}
public class Event{
    int eventId;
    String eventTitle;
    Calendar startDate;
    Calendar endDate;
    int year;
    int month;
    int day;

    public Event(int eventInd, String eventTitle, Calendar startDate, Calendar endDate)
    {
        this.eventId=eventInd;
        this.eventTitle=eventTitle;
        this.startDate=startDate;
        this.endDate=endDate;
        this.year = startDate.get(Calendar.YEAR);
        this.month= startDate.get(Calendar.MONTH);
        this.day = startDate.get(Calendar.DAY_OF_MONTH);
    }
}

Obviously, this is extremely ugly and not at all great way of doing this, but I can't figure out a better way to represent all of this...

This is how the app looks like in action.

  • Dept1 has 4 events
  • Dept2 has 3 events
  • Dept3 has 2 events
  • Dept4 has 1 event

Each Dept has it's own colour as can be seen when toggling them.

\$\endgroup\$
3
\$\begingroup\$

First of all i have a question: How do you know that all the days that have same count also have same colors? For example, day #1 has 3 events from Dept #1, #2, #3, day #2 also has 3 events from Dept #1, #2, #4 so there are 4 colors from two days?

Anyway, looking at the processing that you have, this is how I would do it:

  1. count occurrences for each day: create a Map that holds CalendarDay as key and List of colors as value. the size of the list shows how many times the day appeared in the input.

  2. after iteration on input is complete we have the required information per day. now we iterate over the map entries and build the collections per count. instead of array of ints I would create a Set<Integer> in order to collapse duplicate colors.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Each day would have its own number of colors based on the number of events. Day1 would have colors{1,2,3} Day2 will have colors{1,2,4} I'll add the app demo in about a minute with a bit more clarification. \$\endgroup\$ – Иво Недев Apr 16 '18 at 10:47
  • \$\begingroup\$ oh I see what you mean, good one. \$\endgroup\$ – Иво Недев Apr 16 '18 at 10:51
1
\$\begingroup\$

This basically boils down to a grouping problem: you have an input list and want to group it by date, which yields a per-date list of a given length (1 to number of departments). Then, these list's contents get added to a collector list chosen by the list's size, i.e. add all contents of list with size 1 to collector 1, add all contents of lists with size 2 to collector 2, and so on.

Using the stream API, you already have the grouping, the rest is a little footwork:

// example code uses this data class:
private static class EventThingy {
    public int dept;
    public LocalDate date;

    public EventThingy(int dept, LocalDate date) {
        this.dept = dept;
        this.date = date;
    }
}

Collection<EventThingy> in = ...;

// perform grouping:
Map<LocalDate, List<EventThingy>> grouped = in.stream()
    .collect(Collectors.groupingBy(evt -> evt.date));

// Prepare target list in map identified by count (5 is departmentCount)
// ... or just put your pre-made lists into such a lookup structure instead
Map<Integer, ArrayList<EventThingy>> countToList = IntStream.rangeClosed(1, 5).boxed()
   .collect(Collectors.toMap(Function.identity(), i -> new ArrayList<EventThingy>()));

// Move entries to target lists:
for(List<EventThingy> listAtDate : grouped.values()) {
    ArrayList<EventThingy> targetList = countToList.get(listAtDate.size());
    targetList.addAll(listAtDate);
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

I think @SharonBenAsher's answer is a good starting point for your actual question. I'll instead focus on a few other areas. :)

Declare to the appropriate types, preferring interfaces over implementations

// Not ideal as it's coded against the implementation
ArrayList<CalendarDay> currentDays = filteredEvents.get(i).calDayArr;
// Not ideal as Collection is too generic an interface for the usage here
Collection<CalendarDay> oneEventDays =  new ArrayList<>();    

// Suitable 'middle ground' approach
List<CalendarDay> currentDays = filteredEvents.get(i).calDayArr;
List<CalendarDay> oneEventDays = new ArrayList<>(); 

Don't expose state publicly too easily

CalendarDay.calDayArr is a public field, which is not recommended as you are freely allowing callers of a CalendarDay instance to also freely modify the elements within. Consider a getter getCalendarDays() that returns an unmodifiable List of the state, it will be even better if CalendarDay itself is an immutable class.

public List<CalendarDay> getCalendarDays() {
    return Collections.unmodifiableList(calendarDays);
}

for-each loop

You rely heavily on the regular for (int i = 0; /* ... */) construct for iteration. You can consider the for-each style, especially when you don't require knowing the current index.

for(int i=1;i<filteredEvents.size();i++) {
    ArrayList<CalendarDay> currentDays = filteredEvents.get(i).calDayArr;
    // ...
}

// recommended
for (Filter filter : filteredEvents) {
    List<CalendarDay> currentDays = filter.getCalendarDays();
    // ...
}

Naming and style convention

On a related note, calDayArr can be better named, and you should standardize where your braces are. In fact, use them even for one-liner if statements, as the following looked odd at first glance.

if(eventList!=null)
calDayArr = processCalDays(eventList);

// recommended
if (eventList != null) {
    calendarDays = processCalDays(eventList);
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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