Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am using traceview and DDMS to analyse my operation. The following is my traceview as displayed in DDMS:

enter image description here

According to the above, the LoaderClass.months() function takes about 51% of the CPU time and 798.80 System time. I have reproduced this function below:

public int months(String date) {

        SimpleDateFormat f = new SimpleDateFormat("yyyy-MM-dd");
        Date d1 = null;
        try {
            d1 = f.parse(date);
        } catch (ParseException e) {

            e.printStackTrace();
        }
        Date d2 = new Date();  
        Calendar c1 = Calendar.getInstance();
        c1.setTime(d1);
        Calendar c2 = Calendar.getInstance();
        c2.setTime(d2);
        int diff = 0;
        if (c2.after(c1)) {
            while (c2.after(c1)) {
                c1.add(Calendar.MONTH, 1);
                if (c2.after(c1)) {
                    diff++;
                }
            }
        } else if (c2.before(c1)) {
            while (c2.before(c1)) {
                c1.add(Calendar.MONTH, -1);
                if (c1.before(c2)) {
                    diff--;
                }
            }
        }
        return diff;
    }

Below is a more detailed view of the method as on DDMS:

enter image description here

share|improve this question
    
A month isn't a timespan in the same sense as a week or an hour. With that in mind, how do you define "number of months between 2 dates"? –  Kos Aug 8 at 7:14
    
Number of months between two dates is: (YYYY MM DD): 2001-06-07 to 2002-06-07: Difference would be 12 months exactly, in cases such as 1 month and some days, difference would be only 1 month. –  Skynet Aug 8 at 7:42
    
Post rolled back as it invalidated answers. –  Jamal Aug 11 at 4:44

2 Answers 2

up vote 7 down vote accepted

Don't reinvent the wheel for what Joda time has already written for you:

int monthsBetween = Months.monthsBetween(new LocalDate(date1), new LocalDate()).getMonths();

From the localDate doc:

  1. You can pass a string in the constructor. (you could test that)
  2. Empty constructor is current time.
share|improve this answer
    
I highly regret that I did not use Joda Time. Now I have advanced too much in the project and reverting back is not an option. –  Skynet Aug 8 at 5:32
    
you don't have to revert, add the dependency and this will work, your date1 and date2 are normal java.util.Date –  chillworld Aug 8 at 5:36
    
I will have to make changes in each of my function - to map them to Joda time, that is time consuming. However I will for sure check it out right now. –  Skynet Aug 8 at 5:50
    
Can I invite you to the chat to discuss furter? –  chillworld Aug 8 at 5:52
2  
Did it with JODA, performance optimized by at least 65 percent! Cheers mate! –  Skynet Aug 8 at 12:40

I don't like the naming style you're using. Single and double-letter identifiers are annoying.

Use meaningful names!

Isn't this:

SimpleDateFormat f = new SimpleDateFormat("yyyy-MM-dd");

More enjoyable to read like that?

SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd");

Also, it's not immediately apparent what the method does - months looks more like a variable name than a method, because it doesn't start with a verb.

A more meaningful name could perhaps be getElapsedMonths... and then the xx2 variables could be named xxPresent, and xx1 could be xxSpecified, so getElapsedMonths would calculate the number of months elapsed since a specified date.

share|improve this answer
    
Thanks, I will definitely consider your advise, but the problem here is not with the naming that slows down the function, but the call to add does. It would be a huge help if you have any hints on how to improve that as well. –  Skynet Aug 8 at 5:49

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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