From 4027c9da7b1b998c1c2faffb16b91336191a6062 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 5 May 2017 18:37:09 -0400 Subject: [PATCH] TimeValue#parseTimeValue author is bad, feels bad I stumbled on this code today and I hated it; I wrote it. I did not like that you could not tell at a glance whether or not the method parameters were correct. This commit fixes it. Relates #24522 --- .../elasticsearch/common/unit/TimeValue.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java b/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java index 4c3344eb9d8..0f6eabed1e3 100644 --- a/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java +++ b/core/src/main/java/org/elasticsearch/common/unit/TimeValue.java @@ -254,7 +254,7 @@ public class TimeValue implements Writeable, Comparable { * Returns a {@link String} representation of the current {@link TimeValue}. * * Note that this method might produce fractional time values (ex 1.6m) which cannot be - * parsed by method like {@link TimeValue#parse(String, String, int)}. + * parsed by method like {@link TimeValue#parse(String, String, String)}. */ @Override public String toString() { @@ -326,22 +326,20 @@ public class TimeValue implements Writeable, Comparable { } final String normalized = sValue.toLowerCase(Locale.ROOT).trim(); if (normalized.endsWith("nanos")) { - return new TimeValue(parse(sValue, normalized, 5), TimeUnit.NANOSECONDS); + return new TimeValue(parse(sValue, normalized, "nanos"), TimeUnit.NANOSECONDS); } else if (normalized.endsWith("micros")) { - return new TimeValue(parse(sValue, normalized, 6), TimeUnit.MICROSECONDS); + return new TimeValue(parse(sValue, normalized, "micros"), TimeUnit.MICROSECONDS); } else if (normalized.endsWith("ms")) { - return new TimeValue(parse(sValue, normalized, 2), TimeUnit.MILLISECONDS); + return new TimeValue(parse(sValue, normalized, "ms"), TimeUnit.MILLISECONDS); } else if (normalized.endsWith("s")) { - return new TimeValue(parse(sValue, normalized, 1), TimeUnit.SECONDS); + return new TimeValue(parse(sValue, normalized, "s"), TimeUnit.SECONDS); } else if (sValue.endsWith("m")) { - // parsing minutes should be case sensitive as `M` is generally - // accepted to mean months not minutes. This is the only case where - // the upper and lower case forms indicate different time units - return new TimeValue(parse(sValue, normalized, 1), TimeUnit.MINUTES); + // parsing minutes should be case-sensitive as 'M' means "months", not "minutes"; this is the only special case. + return new TimeValue(parse(sValue, normalized, "m"), TimeUnit.MINUTES); } else if (normalized.endsWith("h")) { - return new TimeValue(parse(sValue, normalized, 1), TimeUnit.HOURS); + return new TimeValue(parse(sValue, normalized, "h"), TimeUnit.HOURS); } else if (normalized.endsWith("d")) { - return new TimeValue(parse(sValue, normalized, 1), TimeUnit.DAYS); + return new TimeValue(parse(sValue, normalized, "d"), TimeUnit.DAYS); } else if (normalized.matches("-0*1")) { return TimeValue.MINUS_ONE; } else if (normalized.matches("0+")) { @@ -355,8 +353,8 @@ public class TimeValue implements Writeable, Comparable { } } - private static long parse(final String initialInput, final String normalized, final int suffixLength) { - final String s = normalized.substring(0, normalized.length() - suffixLength).trim(); + private static long parse(final String initialInput, final String normalized, final String suffix) { + final String s = normalized.substring(0, normalized.length() - suffix.length()).trim(); try { return Long.parseLong(s); } catch (final NumberFormatException e) {