I'd just say, WTF is this doing?
You're testing minutes
and hours
first as if they were the most important things. So with 10 minutes, 1 hours and 1000 days, you output something like "1 hours ago".
Maybe it's impossible as the variables all get computed from a single time interval. But you didn't show us.
I guess it's better for my sanity not to try too hard to figure out what it exactly does. So let me assume that all variable come from a single time interval. Then a condition like
minutes >= 60 && hours < 24
can be expressed like
1 <= hours && hours < 24
This doesn't buy us much as the order is still confusing:
- " hours ago"
- " days ago"
- " minutes from now"
- " months ago"
Again, not trying to find a system there.
What about something like
String suffix = minutes < 0 ? "ago" : "from now";
if (Math.abs(days) >= 365) {
return (Math.abs(days) / 365) + " years " + suffix;
} else if (Math.abs(days) >= 2*30) {
return (Math.abs(days) / 12) + " months " + suffix;
} else if (Math.abs(days) >= 30) {
return days < 0 ? "Last month" : "Next month";
} else if (Math.abs(days) >= 1) {
return Math.abs(days) + " days " + suffix;
} else if (Math.abs(hours) >= 1) {
return Math.abs(hours) + " hours " + suffix;
} else {
return Math.abs(minutes) + " minutes " + suffix;
}
I'm not claiming it's correct, but it's short and damn simple, so you find and fix all bugs in a few seconds.
So what I dislike is
- The high count of cases, which could be cut in half using the
abs
idea stolen from Hosch250. - Placing two tests in each if when one is enough. You need no range tests if you do it systematically.
- Even with range tests, there should be a clean order.
I always prefer to keep things simple from the very beginning. Otherwise, it can easily happen that you get a cool idea, but can't apply it because of the need to preserve compatibility with some quirks in the original.