3
\$\begingroup\$

I'm using Java 7.

private Long difference;
public Long timeDifference(String weboutput) {
    try {
        Calendar calendar = GregorianCalendar.getInstance();
        Calendar today = new GregorianCalendar();
        Date inputTime;
        if (weboutput.length() <= 11) { // for data fetched for current date.
            DateFormat formatter = new SimpleDateFormat("hh:mm:ss a", Locale.US);
            inputTime = formatter.parse(weboutput);
            calendar.setTime(inputTime);
            int hour = calendar.get(Calendar.HOUR);
            int minute = calendar.get(Calendar.MINUTE);
            int second = calendar.get(Calendar.SECOND);
            today.setTime(new Date());
            today.set(Calendar.HOUR, hour);
            today.set(Calendar.MINUTE, minute);
            today.set(Calendar.SECOND, second);
        } else {
            if (weboutput.length() <= 15) { // for data for earlier date in same year or month.
                DateFormat formatter = new SimpleDateFormat("MMM dd hh:mm a", Locale.US);
                inputTime = formatter.parse(weboutput);
                calendar.setTime(inputTime);
                int hour = calendar.get(Calendar.HOUR);
                int minute = calendar.get(Calendar.MINUTE);
                int month = calendar.get(Calendar.MONTH);
                int date = calendar.get(Calendar.DATE);
                today.setTime(new Date());
                today.set(Calendar.HOUR, hour);
                today.set(Calendar.MINUTE, minute);
                today.set(Calendar.MONTH, month);
                today.set(Calendar.DATE, date);
            } else { // for data with different year.
                DateFormat formatter = new SimpleDateFormat("MMM dd, yyyy hh:mm:ss a", Locale.US);
                inputTime = formatter.parse(weboutput);
                calendar.setTime(inputTime);
                int hour = calendar.get(Calendar.HOUR);
                int minute = calendar.get(Calendar.MINUTE);
                int second = calendar.get(Calendar.SECOND);
                int month = calendar.get(Calendar.MONTH);
                int date = calendar.get(Calendar.DATE);
                int year = calendar.get(Calendar.YEAR);
                today.setTime(new Date());
                today.set(Calendar.HOUR, hour);
                today.set(Calendar.MINUTE, minute);
                today.set(Calendar.SECOND, second);
                today.set(Calendar.YEAR, year);
                today.set(Calendar.MONTH, month);
                today.set(Calendar.DATE, date);
            }
        }
        Date retrivedDate = today.getTime();
        Calendar cal = Calendar.getInstance();
        Date currentDate = cal.getTime();
        difference = currentDate.getTime() - retrivedDate.getTime();
        System.out.println(retrivedDate);
        System.out.println(currentDate);
        System.out.println(difference);

    } catch (ParseException e) {
        e.printStackTrace();
    }
    return difference;
}

public boolean alarmValue(Long alarmTime) {
    if (alarmTime <= 1800000) // change this value for Alarm duration, currently 30 min = 30* 60 s = 1800 * 1000 ms = 1800000 ms.
        return false;
    else
        return true;
}
\$\endgroup\$
  • 6
    \$\begingroup\$ Welcome to Code Review! Please include a description at the top before your code, briefly explaining what your code does. Without that, not only it's harder to review, but the summary of your question on the front page shows up as "..." instead of something interesting. See also this meta post to make the most out of asking questions here. Enjoy the ride! \$\endgroup\$ – janos Sep 3 '15 at 8:38
  • \$\begingroup\$ A bit of background would be useful. Where does weboutput come from, and what makes you so certain that it is in one of those three formats? \$\endgroup\$ – 200_success Sep 3 '15 at 12:47
  • \$\begingroup\$ Hi, the output comes from another program calling these class methods. there are three formats in which it can come, data for same date and time as of today, data for same year and earlier data. \$\endgroup\$ – Pranav Nath Sep 8 '15 at 9:57
  • \$\begingroup\$ if (alarmTime <= 1800000) return false; else return true; - ordinary return alarmTime > 1800000 not good enough? ;) \$\endgroup\$ – Konrad Morawski Oct 28 '15 at 11:19
5
\$\begingroup\$

Do you see what I see? Because I see copy-pasta: Yummy :)

            int hour = calendar.get(Calendar.HOUR);
            int minute = calendar.get(Calendar.MINUTE);
            int second = calendar.get(Calendar.SECOND);
            int month = calendar.get(Calendar.MONTH);
            int date = calendar.get(Calendar.DATE);
            int year = calendar.get(Calendar.YEAR);
            today.setTime(new Date());
            today.set(Calendar.HOUR, hour);
            today.set(Calendar.MINUTE, minute);
            today.set(Calendar.SECOND, second);
            today.set(Calendar.YEAR, year);
            today.set(Calendar.MONTH, month);
            today.set(Calendar.DATE, date);

This (or subsections thereof) are copy-pasted for each block in your method.

This screams for extraction into a method:

private static void copyCalendarFields(Calendar source, Calendar destination,
  int... fields) {
    for (int field : fields) {
       destination.set(field, source.get(field));
    }
}

And suddenly things become significantly smaller:

        int hour = calendar.get(Calendar.HOUR);
        int minute = calendar.get(Calendar.MINUTE);
        int second = calendar.get(Calendar.SECOND);
        today.setTime(new Date());
        today.set(Calendar.HOUR, hour);
        today.set(Calendar.MINUTE, minute);
        today.set(Calendar.SECOND, second);

is just:

copyCalendarFields(calendar, today, 
    Calendar.HOUR, Calendar.MINUTE, Calendar.SECOND);

and suddenly these long blocks of code are gone and the whole method is a lot more digestable

\$\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.