Committed by
Gerrit Code Review
Define sub-types of Bandwidth to reduce round-off error
Two sub-types are defined - LongBandwidth - DoubleBandwidth LongBandwidth can reduce round-off error cause by floating point arithmetics. These classes are not exposed outside the package and only instantiated through static factory methods. Change-Id: Ice5d8ff1397c9dd9c8c1fff46af256fff08fa616
Showing
6 changed files
with
254 additions
and
50 deletions
... | @@ -371,7 +371,13 @@ public abstract class ConnectivityIntentCommand extends AbstractShellCommand { | ... | @@ -371,7 +371,13 @@ public abstract class ConnectivityIntentCommand extends AbstractShellCommand { |
371 | 371 | ||
372 | // Check for a bandwidth specification | 372 | // Check for a bandwidth specification |
373 | if (!isNullOrEmpty(bandwidthString)) { | 373 | if (!isNullOrEmpty(bandwidthString)) { |
374 | - final Bandwidth bandwidth = Bandwidth.bps(Double.parseDouble(bandwidthString)); | 374 | + Bandwidth bandwidth; |
375 | + try { | ||
376 | + bandwidth = Bandwidth.bps(Long.parseLong(bandwidthString)); | ||
377 | + // when the string can't be parsed as long, then try to parse as double | ||
378 | + } catch (NumberFormatException e) { | ||
379 | + bandwidth = Bandwidth.bps(Double.parseDouble(bandwidthString)); | ||
380 | + } | ||
375 | constraints.add(new BandwidthConstraint(bandwidth)); | 381 | constraints.add(new BandwidthConstraint(bandwidth)); |
376 | } | 382 | } |
377 | 383 | ... | ... |
... | @@ -267,6 +267,8 @@ public final class KryoNamespaces { | ... | @@ -267,6 +267,8 @@ public final class KryoNamespaces { |
267 | .register(VlanId.class) | 267 | .register(VlanId.class) |
268 | .register(Frequency.class) | 268 | .register(Frequency.class) |
269 | .register(Bandwidth.class) | 269 | .register(Bandwidth.class) |
270 | + .register(Bandwidth.bps(1L).getClass()) | ||
271 | + .register(Bandwidth.bps(1.0).getClass()) | ||
270 | .build(); | 272 | .build(); |
271 | 273 | ||
272 | /** | 274 | /** |
... | @@ -283,7 +285,7 @@ public final class KryoNamespaces { | ... | @@ -283,7 +285,7 @@ public final class KryoNamespaces { |
283 | .register(BASIC) | 285 | .register(BASIC) |
284 | .nextId(KryoNamespace.INITIAL_ID + 30) | 286 | .nextId(KryoNamespace.INITIAL_ID + 30) |
285 | .register(MISC) | 287 | .register(MISC) |
286 | - .nextId(KryoNamespace.INITIAL_ID + 30 + 10) | 288 | + .nextId(KryoNamespace.INITIAL_ID + 30 + 20) |
287 | .register( | 289 | .register( |
288 | Instructions.MeterInstruction.class, | 290 | Instructions.MeterInstruction.class, |
289 | MeterId.class, | 291 | MeterId.class, | ... | ... |
... | @@ -397,6 +397,7 @@ public class KryoSerializerTest { | ... | @@ -397,6 +397,7 @@ public class KryoSerializerTest { |
397 | 397 | ||
398 | @Test | 398 | @Test |
399 | public void testBandwidth() { | 399 | public void testBandwidth() { |
400 | + testSerializedEquals(Bandwidth.mbps(1000)); | ||
400 | testSerializedEquals(Bandwidth.mbps(1000.0)); | 401 | testSerializedEquals(Bandwidth.mbps(1000.0)); |
401 | } | 402 | } |
402 | 403 | ... | ... |
... | @@ -17,28 +17,21 @@ package org.onlab.util; | ... | @@ -17,28 +17,21 @@ package org.onlab.util; |
17 | 17 | ||
18 | import com.google.common.collect.ComparisonChain; | 18 | import com.google.common.collect.ComparisonChain; |
19 | 19 | ||
20 | -import java.util.Objects; | ||
21 | - | ||
22 | /** | 20 | /** |
23 | * Representation of bandwidth. | 21 | * Representation of bandwidth. |
24 | * Use the static factory method corresponding to the unit (like Kbps) you desire on instantiation. | 22 | * Use the static factory method corresponding to the unit (like Kbps) you desire on instantiation. |
25 | */ | 23 | */ |
26 | -public final class Bandwidth implements RichComparable<Bandwidth> { | 24 | +public interface Bandwidth extends RichComparable<Bandwidth> { |
27 | - | ||
28 | - private final double bps; | ||
29 | 25 | ||
30 | /** | 26 | /** |
31 | * Creates a new instance with given bandwidth. | 27 | * Creates a new instance with given bandwidth. |
32 | * | 28 | * |
33 | - * @param bps bandwidth value to be assigned | 29 | + * @param v bandwidth value |
30 | + * @param unit {@link DataRateUnit} of {@code v} | ||
31 | + * @return {@link Bandwidth} instance with given bandwidth | ||
34 | */ | 32 | */ |
35 | - private Bandwidth(double bps) { | 33 | + static Bandwidth of(long v, DataRateUnit unit) { |
36 | - this.bps = bps; | 34 | + return new LongBandwidth(unit.toBitsPerSecond(v)); |
37 | - } | ||
38 | - | ||
39 | - // Constructor for serialization | ||
40 | - private Bandwidth() { | ||
41 | - this.bps = 0; | ||
42 | } | 35 | } |
43 | 36 | ||
44 | /** | 37 | /** |
... | @@ -48,8 +41,8 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -48,8 +41,8 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
48 | * @param unit {@link DataRateUnit} of {@code v} | 41 | * @param unit {@link DataRateUnit} of {@code v} |
49 | * @return {@link Bandwidth} instance with given bandwidth | 42 | * @return {@link Bandwidth} instance with given bandwidth |
50 | */ | 43 | */ |
51 | - public static Bandwidth of(double v, DataRateUnit unit) { | 44 | + static Bandwidth of(double v, DataRateUnit unit) { |
52 | - return new Bandwidth(unit.toBitsPerSecond(v)); | 45 | + return new DoubleBandwidth(unit.toBitsPerSecond(v)); |
53 | } | 46 | } |
54 | 47 | ||
55 | /** | 48 | /** |
... | @@ -58,8 +51,18 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -58,8 +51,18 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
58 | * @param bps bandwidth value to be assigned | 51 | * @param bps bandwidth value to be assigned |
59 | * @return {@link Bandwidth} instance with given bandwidth | 52 | * @return {@link Bandwidth} instance with given bandwidth |
60 | */ | 53 | */ |
61 | - public static Bandwidth bps(double bps) { | 54 | + static Bandwidth bps(long bps) { |
62 | - return new Bandwidth(bps); | 55 | + return new LongBandwidth(bps); |
56 | + } | ||
57 | + | ||
58 | + /** | ||
59 | + * Creates a new instance with given bandwidth in bps. | ||
60 | + * | ||
61 | + * @param bps bandwidth value to be assigned | ||
62 | + * @return {@link Bandwidth} instance with given bandwidth | ||
63 | + */ | ||
64 | + static Bandwidth bps(double bps) { | ||
65 | + return new DoubleBandwidth(bps); | ||
63 | } | 66 | } |
64 | 67 | ||
65 | /** | 68 | /** |
... | @@ -68,7 +71,17 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -68,7 +71,17 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
68 | * @param kbps bandwidth value to be assigned | 71 | * @param kbps bandwidth value to be assigned |
69 | * @return {@link Bandwidth} instance with given bandwidth | 72 | * @return {@link Bandwidth} instance with given bandwidth |
70 | */ | 73 | */ |
71 | - public static Bandwidth kbps(double kbps) { | 74 | + static Bandwidth kbps(long kbps) { |
75 | + return bps(kbps * 1_000L); | ||
76 | + } | ||
77 | + | ||
78 | + /** | ||
79 | + * Creates a new instance with given bandwidth in Kbps. | ||
80 | + * | ||
81 | + * @param kbps bandwidth value to be assigned | ||
82 | + * @return {@link Bandwidth} instance with given bandwidth | ||
83 | + */ | ||
84 | + static Bandwidth kbps(double kbps) { | ||
72 | return bps(kbps * 1_000L); | 85 | return bps(kbps * 1_000L); |
73 | } | 86 | } |
74 | 87 | ||
... | @@ -78,17 +91,37 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -78,17 +91,37 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
78 | * @param mbps bandwidth value to be assigned | 91 | * @param mbps bandwidth value to be assigned |
79 | * @return {@link Bandwidth} instance with given bandwidth | 92 | * @return {@link Bandwidth} instance with given bandwidth |
80 | */ | 93 | */ |
81 | - public static Bandwidth mbps(double mbps) { | 94 | + static Bandwidth mbps(long mbps) { |
82 | return bps(mbps * 1_000_000L); | 95 | return bps(mbps * 1_000_000L); |
83 | } | 96 | } |
84 | 97 | ||
85 | /** | 98 | /** |
99 | + * Creates a new instance with given bandwidth in Mbps. | ||
100 | + * | ||
101 | + * @param mbps bandwidth value to be assigned | ||
102 | + * @return {@link Bandwidth} instance with given bandwidth | ||
103 | + */ | ||
104 | + static Bandwidth mbps(double mbps) { | ||
105 | + return bps(mbps * 1_000_000L); | ||
106 | + } | ||
107 | + | ||
108 | + /** | ||
109 | + * Creates a new instance with given bandwidth in Gbps. | ||
110 | + * | ||
111 | + * @param gbps bandwidth value to be assigned | ||
112 | + * @return {@link Bandwidth} instance with given bandwidth | ||
113 | + */ | ||
114 | + static Bandwidth gbps(long gbps) { | ||
115 | + return bps(gbps * 1_000_000_000L); | ||
116 | + } | ||
117 | + | ||
118 | + /** | ||
86 | * Creates a new instance with given bandwidth in Gbps. | 119 | * Creates a new instance with given bandwidth in Gbps. |
87 | * | 120 | * |
88 | * @param gbps bandwidth value to be assigned | 121 | * @param gbps bandwidth value to be assigned |
89 | * @return {@link Bandwidth} instance with given bandwidth | 122 | * @return {@link Bandwidth} instance with given bandwidth |
90 | */ | 123 | */ |
91 | - public static Bandwidth gbps(double gbps) { | 124 | + static Bandwidth gbps(double gbps) { |
92 | return bps(gbps * 1_000_000_000L); | 125 | return bps(gbps * 1_000_000_000L); |
93 | } | 126 | } |
94 | 127 | ||
... | @@ -97,9 +130,7 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -97,9 +130,7 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
97 | * | 130 | * |
98 | * @return bandwidth in bps. | 131 | * @return bandwidth in bps. |
99 | */ | 132 | */ |
100 | - public double bps() { | 133 | + double bps(); |
101 | - return bps; | ||
102 | - } | ||
103 | 134 | ||
104 | /** | 135 | /** |
105 | * Returns a Bandwidth whose value is (this + value). | 136 | * Returns a Bandwidth whose value is (this + value). |
... | @@ -107,8 +138,8 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -107,8 +138,8 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
107 | * @param value value to be added to this Frequency | 138 | * @param value value to be added to this Frequency |
108 | * @return this + value | 139 | * @return this + value |
109 | */ | 140 | */ |
110 | - public Bandwidth add(Bandwidth value) { | 141 | + default Bandwidth add(Bandwidth value) { |
111 | - return bps(this.bps + value.bps); | 142 | + return bps(this.bps() + value.bps()); |
112 | } | 143 | } |
113 | 144 | ||
114 | /** | 145 | /** |
... | @@ -117,33 +148,14 @@ public final class Bandwidth implements RichComparable<Bandwidth> { | ... | @@ -117,33 +148,14 @@ public final class Bandwidth implements RichComparable<Bandwidth> { |
117 | * @param value value to be added to this Frequency | 148 | * @param value value to be added to this Frequency |
118 | * @return this - value | 149 | * @return this - value |
119 | */ | 150 | */ |
120 | - public Bandwidth subtract(Bandwidth value) { | 151 | + default Bandwidth subtract(Bandwidth value) { |
121 | - return bps(this.bps - value.bps); | 152 | + return bps(this.bps() - value.bps()); |
122 | } | 153 | } |
123 | 154 | ||
124 | @Override | 155 | @Override |
125 | - public int compareTo(Bandwidth other) { | 156 | + default int compareTo(Bandwidth other) { |
126 | return ComparisonChain.start() | 157 | return ComparisonChain.start() |
127 | - .compare(this.bps, other.bps) | 158 | + .compare(this.bps(), other.bps()) |
128 | .result(); | 159 | .result(); |
129 | } | 160 | } |
130 | - | ||
131 | - @Override | ||
132 | - public boolean equals(Object obj) { | ||
133 | - if (obj instanceof Bandwidth) { | ||
134 | - Bandwidth that = (Bandwidth) obj; | ||
135 | - return Objects.equals(this.bps, that.bps); | ||
136 | - } | ||
137 | - return false; | ||
138 | - } | ||
139 | - | ||
140 | - @Override | ||
141 | - public int hashCode() { | ||
142 | - return Long.hashCode(Double.doubleToLongBits(bps)); | ||
143 | - } | ||
144 | - | ||
145 | - @Override | ||
146 | - public String toString() { | ||
147 | - return String.valueOf(this.bps); | ||
148 | - } | ||
149 | } | 161 | } | ... | ... |
1 | +/* | ||
2 | + * Copyright 2016 Open Networking Laboratory | ||
3 | + * | ||
4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | + * you may not use this file except in compliance with the License. | ||
6 | + * You may obtain a copy of the License at | ||
7 | + * | ||
8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | + * | ||
10 | + * Unless required by applicable law or agreed to in writing, software | ||
11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | + * See the License for the specific language governing permissions and | ||
14 | + * limitations under the License. | ||
15 | + */ | ||
16 | +package org.onlab.util; | ||
17 | + | ||
18 | +import java.util.Objects; | ||
19 | + | ||
20 | +/** | ||
21 | + * Representation of bandwidth. | ||
22 | + * Use the static factory method corresponding to the unit (like Kbps) you desire on instantiation. | ||
23 | + */ | ||
24 | +final class DoubleBandwidth implements Bandwidth { | ||
25 | + | ||
26 | + private final double bps; | ||
27 | + | ||
28 | + /** | ||
29 | + * Creates a new instance with given bandwidth. | ||
30 | + * | ||
31 | + * @param bps bandwidth value to be assigned | ||
32 | + */ | ||
33 | + DoubleBandwidth(double bps) { | ||
34 | + this.bps = bps; | ||
35 | + } | ||
36 | + | ||
37 | + // Constructor for serialization | ||
38 | + private DoubleBandwidth() { | ||
39 | + this.bps = 0; | ||
40 | + } | ||
41 | + /** | ||
42 | + * Returns bandwidth in bps. | ||
43 | + * | ||
44 | + * @return bandwidth in bps. | ||
45 | + */ | ||
46 | + public double bps() { | ||
47 | + return bps; | ||
48 | + } | ||
49 | + | ||
50 | + @Override | ||
51 | + public boolean equals(Object obj) { | ||
52 | + if (obj == this) { | ||
53 | + return true; | ||
54 | + } | ||
55 | + | ||
56 | + if (!(obj instanceof DoubleBandwidth)) { | ||
57 | + return false; | ||
58 | + } | ||
59 | + | ||
60 | + DoubleBandwidth that = (DoubleBandwidth) obj; | ||
61 | + return Objects.equals(this.bps, that.bps); | ||
62 | + } | ||
63 | + | ||
64 | + @Override | ||
65 | + public int hashCode() { | ||
66 | + return Long.hashCode(Double.doubleToLongBits(bps)); | ||
67 | + } | ||
68 | + | ||
69 | + @Override | ||
70 | + public String toString() { | ||
71 | + return String.valueOf(this.bps); | ||
72 | + } | ||
73 | +} |
1 | +/* | ||
2 | + * Copyright 2016 Open Networking Laboratory | ||
3 | + * | ||
4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
5 | + * you may not use this file except in compliance with the License. | ||
6 | + * You may obtain a copy of the License at | ||
7 | + * | ||
8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
9 | + * | ||
10 | + * Unless required by applicable law or agreed to in writing, software | ||
11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
13 | + * See the License for the specific language governing permissions and | ||
14 | + * limitations under the License. | ||
15 | + */ | ||
16 | +package org.onlab.util; | ||
17 | + | ||
18 | +import com.google.common.collect.ComparisonChain; | ||
19 | + | ||
20 | +/** | ||
21 | + * Representation of bandwidth. | ||
22 | + * Use the static factory method corresponding to the unit (like Kbps) you desire on instantiation. | ||
23 | + */ | ||
24 | +final class LongBandwidth implements Bandwidth { | ||
25 | + | ||
26 | + private final long bps; | ||
27 | + | ||
28 | + /** | ||
29 | + * Creates a new instance with given bandwidth. | ||
30 | + * | ||
31 | + * @param bps bandwidth value to be assigned | ||
32 | + */ | ||
33 | + LongBandwidth(long bps) { | ||
34 | + this.bps = bps; | ||
35 | + } | ||
36 | + | ||
37 | + // Constructor for serialization | ||
38 | + private LongBandwidth() { | ||
39 | + this.bps = 0; | ||
40 | + } | ||
41 | + /** | ||
42 | + * Returns bandwidth in bps. | ||
43 | + * | ||
44 | + * @return bandwidth in bps. | ||
45 | + */ | ||
46 | + public double bps() { | ||
47 | + return bps; | ||
48 | + } | ||
49 | + | ||
50 | + /** | ||
51 | + * Returns a Bandwidth whose value is (this + value). | ||
52 | + * | ||
53 | + * @param value value to be added to this Frequency | ||
54 | + * @return this + value | ||
55 | + */ | ||
56 | + public Bandwidth add(Bandwidth value) { | ||
57 | + if (value instanceof LongBandwidth) { | ||
58 | + return Bandwidth.bps(this.bps + ((LongBandwidth) value).bps); | ||
59 | + } | ||
60 | + return Bandwidth.bps(this.bps + value.bps()); | ||
61 | + } | ||
62 | + | ||
63 | + /** | ||
64 | + * Returns a Bandwidth whose value is (this - value). | ||
65 | + * | ||
66 | + * @param value value to be added to this Frequency | ||
67 | + * @return this - value | ||
68 | + */ | ||
69 | + public Bandwidth subtract(Bandwidth value) { | ||
70 | + if (value instanceof LongBandwidth) { | ||
71 | + return Bandwidth.bps(this.bps - ((LongBandwidth) value).bps); | ||
72 | + } | ||
73 | + return Bandwidth.bps(this.bps - value.bps()); | ||
74 | + } | ||
75 | + | ||
76 | + @Override | ||
77 | + public int compareTo(Bandwidth other) { | ||
78 | + if (other instanceof LongBandwidth) { | ||
79 | + return ComparisonChain.start() | ||
80 | + .compare(this.bps, ((LongBandwidth) other).bps) | ||
81 | + .result(); | ||
82 | + } | ||
83 | + return ComparisonChain.start() | ||
84 | + .compare(this.bps, other.bps()) | ||
85 | + .result(); | ||
86 | + } | ||
87 | + | ||
88 | + @Override | ||
89 | + public boolean equals(Object obj) { | ||
90 | + if (obj == this) { | ||
91 | + return true; | ||
92 | + } | ||
93 | + | ||
94 | + if (obj instanceof Bandwidth) { | ||
95 | + return this.compareTo((Bandwidth) obj) == 0; | ||
96 | + } | ||
97 | + | ||
98 | + return false; | ||
99 | + } | ||
100 | + | ||
101 | + @Override | ||
102 | + public int hashCode() { | ||
103 | + return Long.hashCode(bps); | ||
104 | + } | ||
105 | + | ||
106 | + @Override | ||
107 | + public String toString() { | ||
108 | + return String.valueOf(this.bps); | ||
109 | + } | ||
110 | +} |
-
Please register or login to post a comment