views:

101

answers:

2

Hey Everyone,

I was wondering if anyone could look over a class I wrote, I am receiving generic warnings in Eclipse and I am just wondering if it could be cleaned up at all. All of the warnings I received are surrounded in ** in my code below.

The class takes a list of strings in the form of (hh:mm AM/PM) and converts them into HourMinute objects in order to find the first time in the list that comes after the current time.

I am also curious about if there are more efficient ways to do this. This works fine but the student in me just wants to find out how I could do this better.

public class FindTime {
    private String[] hourMinuteStringArray;

    public FindTime(String[] hourMinuteStringArray){
        this.hourMinuteStringArray = hourMinuteStringArray;
    }

    public int findTime(){

        HourMinuteList hourMinuteList = convertHMStringArrayToHMArray(hourMinuteStringArray);
        Calendar calendar = new GregorianCalendar();
        int hour = calendar.get(Calendar.HOUR_OF_DAY);
        int minute = calendar.get(Calendar.MINUTE);
        HourMinute now = new HourMinute(hour,minute);
        int nearestTimeIndex = findNearestTimeIndex(hourMinuteList, now);
        return nearestTimeIndex;
    }

    private int findNearestTimeIndex(HourMinuteList hourMinuteList, HourMinute now){
        HourMinute current;
        int position = 0;
        Iterator<HourMinute> iterator = **hourMinuteList.iterator()**;
        while(iterator.hasNext()){
            current = (HourMinute) iterator.next();
            if(now.compareTo(current) == -1){
                return position;
            }
            position++;
        }
        return position;
    }


    private static HourMinuteList convertHMStringArrayToHMArray(String[] times){
        FindTime s = new FindTime(new String[1]);
        HourMinuteList list = s.new HourMinuteList();
        String[] splitTime = new String[3];
        for(String time : times ){
            String[] tempFirst = time.split(":");
            String[] tempSecond = tempFirst[1].split(" ");
            splitTime[0] = tempFirst[0];
            splitTime[1] = tempSecond[0];
            splitTime[2] = tempSecond[1];
            int hour = Integer.parseInt(splitTime[0]);
            int minute = Integer.parseInt(splitTime[1]);
            HourMinute hm;
            if(splitTime[2] == "AM"){
                hm = s.new HourMinute(hour,minute);
            }
            else if((splitTime[2].equals("PM")) && (hour < 12)){
                hm = s.new HourMinute(hour + 12,minute);
            }
            else{
                hm = s.new HourMinute(hour,minute);
            }

            **list.add(hm);**
        }
        return list;
    }
    class **HourMinuteList** extends **ArrayList** implements RandomAccess{

    }
    class HourMinute implements **Comparable** {
        int hour;
        int minute;

        public HourMinute(int hour, int minute) {
            setHour(hour);
            setMinute(minute);
        }

        int getMinute() {
            return this.minute;
        }
        String getMinuteString(){
            if(this.minute < 10){
                return "0" + this.minute;
            }else{
                return "" + this.minute;
            }
        }

        int getHour() {
            return this.hour;
        }

        void setHour(int hour) {
            this.hour = hour;
        }

        void setMinute(int minute) {
            this.minute = minute;
        }

        @Override
        public int compareTo(Object aThat) {

            if (aThat instanceof HourMinute) {
                HourMinute that = (HourMinute) aThat;
                if (this.getHour() == that.getHour()) {
                    if (this.getMinute() > that.getMinute()) {
                        return 1;
                    } else if (this.getMinute() < that.getMinute()) {
                        return -1;
                    } else {
                        return 0;
                    }
                } else if (this.getHour() > that.getHour()) {
                    return 1;
                } else if (this.getHour() < that.getHour()) {
                    return -1;
                } else {
                    return 0;
                }
            }

            return 0;
        }

    }


If you have any questions let me know. 

Thanks, 
Rob
+4  A: 

I wouldn't use the HourMinute class, unless it has some other added value. If you only need to find the closest event time after a given point in time, convert your strings to Date (or to long values representing time), and store them in some sorted collection. The conversion can be done with SimpleDateFormat.

If items are added dynamically, use TreeSet<Date>, together with ceiling(t) / higher(t) methods.

If the set of items is not dynamic, use an array Date[], together with Arrays.binarySearch(..).

Here is a (working) draft of the first approach:

public class TimedEventsMgr {
    private TreeSet<Date> pointsInTime = new TreeSet<Date>();
    private SimpleDateFormat sdf = new SimpleDateFormat("yyyyMMdd hh:mm a");

    //timeStr: hh:mm AM/PM
    public void add(String timeStr) throws ParseException{
        Date time = sdf.parse("20000101 "+timeStr);
        pointsInTime.add(time);
    }

    public Date closestFutureTime(Date time){
        Calendar c = Calendar.getInstance();
        c.setTime(time);
        c.set(Calendar.YEAR, 2000);
        c.set(Calendar.MONTH, 0); //January
        c.set(Calendar.DATE, 1);
        return pointsInTime.higher(c.getTime());
    }
}
Eyal Schneider
Alright, I may need to consider doing this. The items are already sorted but I like the idea of using the Arrays.binarySearch(). Do you think there would be any considerable efficiency difference?
Tarmon
@Tarmon: yes, there is a significant efficiency difference. Your code returns the closest item in linear time, while the alternative above does it in logarithmic time.
Eyal Schneider
Very cool. I appreciate the example. I am going to go ahead and play around with this.
Tarmon
+5  A: 

It's because you a not specify generics for your List and Comparable instances, that can support generics. You can rewrite your code with:

class HourMinuteList extends ArrayList<HourMinute> implements RandomAccess{

}
class HourMinute implements Comparable<HourMinute> {

   public int compareTo(HourMinute aThat) {
   ....
  }
}

Note: generics is not required, and not used at runtime, but it's better to use them because it helps you to avoid some bugs at your code.

splix
Thanks, I thought it was something similar to this but I wasn't one hundred percent sure.
Tarmon
Hey Splix, I really appreciate your help but I think I am going to explore the option Eyal presented which kind of makes the generics part of this unnecessary. I guess what I am trying to say is I wish I could accept both answers.
Tarmon