Fix conversion issues in MutableOptions (#9194)
Summary: Removing unnecessary checks around conversion from int/long to double as it does not lose information (see https://docs.oracle.com/javase/specs/jls/se9/html/jls-5.html#jls-5.1.2). For example, `value > Double.MAX_VALUE` is always false when value is long or int. Can you please have a look adamretter? Also fixed some other minor issues (do you prefer a separate PR?) Pull Request resolved: https://github.com/facebook/rocksdb/pull/9194 Reviewed By: ajkr Differential Revision: D36221694 fbshipit-source-id: bf327c07386560b87ddc0c98039e8d6e8f2f1e82
This commit is contained in:
parent
89571b30e5
commit
4527bb2fed
@ -81,8 +81,8 @@ public abstract class AbstractMutableOptions {
|
|||||||
protected abstract T build(final String[] keys, final String[] values);
|
protected abstract T build(final String[] keys, final String[] values);
|
||||||
|
|
||||||
public T build() {
|
public T build() {
|
||||||
final String keys[] = new String[options.size()];
|
final String[] keys = new String[options.size()];
|
||||||
final String values[] = new String[options.size()];
|
final String[] values = new String[options.size()];
|
||||||
|
|
||||||
int i = 0;
|
int i = 0;
|
||||||
for (final Map.Entry<K, MutableOptionValue<?>> option : options.entrySet()) {
|
for (final Map.Entry<K, MutableOptionValue<?>> option : options.entrySet()) {
|
||||||
@ -227,7 +227,7 @@ public abstract class AbstractMutableOptions {
|
|||||||
} catch (NumberFormatException nfe) {
|
} catch (NumberFormatException nfe) {
|
||||||
final double doubleValue = Double.parseDouble(value);
|
final double doubleValue = Double.parseDouble(value);
|
||||||
if (doubleValue != Math.round(doubleValue))
|
if (doubleValue != Math.round(doubleValue))
|
||||||
throw new IllegalArgumentException("Unable to parse or round " + value + " to int");
|
throw new IllegalArgumentException("Unable to parse or round " + value + " to long");
|
||||||
return Math.round(doubleValue);
|
return Math.round(doubleValue);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -245,7 +245,7 @@ public abstract class AbstractMutableOptions {
|
|||||||
} catch (NumberFormatException nfe) {
|
} catch (NumberFormatException nfe) {
|
||||||
final double doubleValue = Double.parseDouble(value);
|
final double doubleValue = Double.parseDouble(value);
|
||||||
if (doubleValue != Math.round(doubleValue))
|
if (doubleValue != Math.round(doubleValue))
|
||||||
throw new IllegalArgumentException("Unable to parse or round " + value + " to long");
|
throw new IllegalArgumentException("Unable to parse or round " + value + " to int");
|
||||||
return (int) Math.round(doubleValue);
|
return (int) Math.round(doubleValue);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -17,7 +17,7 @@ public abstract class MutableOptionValue<T> {
|
|||||||
extends MutableOptionValue<T> {
|
extends MutableOptionValue<T> {
|
||||||
protected final T value;
|
protected final T value;
|
||||||
|
|
||||||
private MutableOptionValueObject(final T value) {
|
protected MutableOptionValueObject(final T value) {
|
||||||
this.value = value;
|
this.value = value;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -153,10 +153,6 @@ public abstract class MutableOptionValue<T> {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
double asDouble() {
|
double asDouble() {
|
||||||
if(value > Double.MAX_VALUE || value < Double.MIN_VALUE) {
|
|
||||||
throw new NumberFormatException(
|
|
||||||
"long value lies outside the bounds of int");
|
|
||||||
}
|
|
||||||
return Long.valueOf(value).doubleValue();
|
return Long.valueOf(value).doubleValue();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -210,9 +206,6 @@ public abstract class MutableOptionValue<T> {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
double asDouble() {
|
double asDouble() {
|
||||||
if(value > Double.MAX_VALUE || value < Double.MIN_VALUE) {
|
|
||||||
throw new NumberFormatException("int value lies outside the bounds of int");
|
|
||||||
}
|
|
||||||
return Integer.valueOf(value).doubleValue();
|
return Integer.valueOf(value).doubleValue();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user