Introduction
I want to check whether number is in given range. Function takes:
- long number that should be checked
- 2 longs threshold values
- byte range mode
Range mode is represented as number of type byte that can be from 0 to 4
- 0 for 00 => exclusive check
- 3 for 11 => inclusive check
- 1 for 01 => maximum - inclusive, minimum - exclusive
- 2 for 10 => maximum - exclusive, minimum - inclusive
Code
Range check constant flags
public static final byte RANGE_EXCLUSIVE = 0; // 00
public static final byte RANGE_INCLUSIVE = 3; // 11
public static final byte RANGE_MAX_INCLUSIVE = 1; // 01
public static final byte RANGE_MIN_INCLUSIVE = 2; // 10
Function
public static boolean inRange(long value, long range_a, long range_b, byte checkMode) {
if (range_a > range_b) {
long tmp = range_a;
range_a = range_b;
range_b = tmp;
}
// If value is on the left edge - check if it is inclusive
if (value == range_a && (checkMode & RANGE_MIN_INCLUSIVE) == 0) return false;
// If value is on the right edge - check if it is inclusive
if (value == range_b && (checkMode & RANGE_MAX_INCLUSIVE) == 0) return false;
// If value is out of range - return false
if (value < range_a) return false;
if (value > range_b) return false;
return true;
}
Question
What do you think about this code?
Should I throw exceptions if range_a is less than range_b?
Or I can return false because range_a is expected to be less or equal to range_b
My first(and current) variant was to swap their values so range_a is always minimum, and range_b is always maximum of them
About style
I'm using camelcase for methods and variables
Underscore is used to show that values are connected. and used together:
Example: pos_x, pos_y or range_a, range_b
2 Answers 2
I fully concur with @TimothyTruckle on format, follow his advice.
Using binaries is restrictive (only two states), and not user-friendly. I don't like byte arithmetics, its quite un-Java like. I suppose you come from C/C++ background.
In more Java-like syntax, I propose a fluent interface on a Range
class:
public class Range {
private static enum RangeType{ INCLUSIVE, EXCLUSIVE, INFINITE };
private static class RangeStart {
private final RangeType startType;
private final Double startValue;
private RangeStart(RangeType type, Double value){
this.startType = type;
this.startValue = value;
}
public Range toInclusive(double endValue) {
// Optionally check endValue >= startValue and throw something
return new Range(startType, startValue, RangeType.INCLUSIVE, endValue);
}
public Range toExclusive(double endValue) {
// Optionally check endValue >= startValue and throw something
return new Range(startType, startValue, RangeType.EXCLUSIVE, endValue);
}
public Range toInfinity() {
return new Range(startType, startValue, RangeType.INFINITY, 0.);
}
}
public static RangeStart fromInfinity() {
retur new RangeStart(RangeType.INFINITY, 0.)
}
public static RangeStart fromInclusive(double fromValue) {
retur new RangeStart(RangeType.INCLUSIVE, fromValue)
}
public static RangeStart fromExclusive(double fromValue) {
retur new RangeStart(RangeType.EXCLUSIVE, fromValue)
}
private final RangeType startType;
private final Double startValue;
private final RangeType endType;
private final Double endValue;
private Range(RangeType startType, Double startValue, RangeType endType, Double endValue) {
this.startType = startType;
this.startValue = startValue;
this.endType = endType;
this.endValue = endValue;
}
public boolean contains(double value){
// Do your thing here, I'm not doing all the work! ;-)
}
}
This creates a re-usable Range object to check values. The Pattern used is similar to the Builder Pattern (I named the Builder RangeStart
).
Example usage:
Range range5i7e = Range.fromInclusive(5).toExclusive(7);
range5i7e.contains(5); // true
range5i7e.contains(5); // false
Range.fromInclusive(-10).toInfinity().contains(3720); // true Range.fromInclusive(5).toExclusive(3); // Throws something ?
Style improvements:
You may want to make inclusive the standard and use Range.from(1).to(2)
to make a closed Range
. It's much less verbose.
You could also make the Range infinite-ended on both sides unless otherwise told, like Range.to(5)
would be ]Inf, 5]
etc.
Should I throw exceptions if range_a is less than range_b?
This depends on your requirements. - Does the user of this function have to provide the input in correct order because a wrong order is a follow up of serious problem in her preceding code? Then you should throw an exception and this might even be a checked custom type to make the user aware of it. - Or is the wrong order something that is likely to happen and no problem outside your method? The reordering and going on inside your method is appropriate.
Should I throw exceptions if range_a is less than range_b?
This also must be specified by the requirements. If there was no requirement add a Javadoc comment explaining that and why you (not) return true
in this situation.
I'm using camelcase for methods and variables
as expected by the java naming conventions.
Underscore is used to show that values are connected. and used together:
Example: pos_x, pos_y or range_a, range_b
Why not posX
, posY
and rangeX
, rangeY
or even better: since Java is an object oriented language, why not using objects to group things that belong together?
class Tupel {
long a,b;
Tupel(int a, int b){
this.a = a;
this.b = b;
}
}
Tupel pos = new Tupel(10,30);
Tupel range = new Tupe(5,59);
// ...
if (range.a > range.b) {
range = new Tupel(range.b,range.a);
// ...
Why isn't checkMode an enumeration? – Alexander
The good thing about Java enums is that they can hold logic. The enum constants are classes that extend the enum class. This way enums can improve your code by resolving if
/else
cascades or switch
constructs with polymorphism.
You don't need to access the enum's raw values.[...] – Alexander
With that in mind you have only 2 enum
constants
enum CompareMode{
RANGE_EXCLUSIVE{
@Override
protected abstract boolean isInRange(long value, long range_a, long range_b){
return value > range_a && value < range_b;
}
},
RANGE_INCLUSIVE{
@Override
protected abstract boolean isInRange(long value, long range_a, long range_b){
return value >= range_a && value =< range_b;
}
};
public boolean inRange(long value, long range_a, long range_b){
if(range_a > range_b)
return isInRange( value,range_b,range_a);
else // <- not needed but her for readability
return isInRange( value,range_a,range_b);
};
}
you complete logic is inside the enum, no extra method needed.
Now lets join that with the Tupel
class:
enum CompareMode{
RANGE_EXCLUSIVE{
@Override
protected abstract boolean isInRange(long value, Tupel range){
return value > range.a && value < range.b;
}
},
RANGE_INCLUSIVE{
@Override
protected abstract boolean isInRange(long value, Tupel range){
return value >= range.a && value =< range.b;
}
};
public boolean inRange(long value, Tupel range){
if(range.a > range.b)
range=new Tupel(range.b,range.a);
return isInRange( value,range_a,range_b);
};
}
The benefit of that solution is:
- the parameter list is shorter and has no more possibility to mix the values passed in accidentally.
- if you later add another CompareMode (which is hard to imaging, but try...) the only thing you have to change is the lines for the new mode and the code where you decide which mode to use. The code that actually needs the result does not need to change.
Also, how are you calling methods in enum? – Michael
like this:
Tupel range = new Tupel(4,5);
for(CompareMode compareMode : CompareMode.values()){
System.out.println(4+" is in range with compareMode "+compareMode +" ="+compareMode.inRange(4,range);
System.out.println(5+" is in range with compareMode "+compareMode +" ="+compareMode.inRange(5,range);
}
-
-
\$\begingroup\$ So, it will be 4 methods for each check type. Also, how are you calling methods in enum? \$\endgroup\$Michael– Michael2017年01月24日 15:06:27 +00:00Commented Jan 24, 2017 at 15:06
-
\$\begingroup\$ @Michael "So, it will be 4 methods for each check type. " hwo do you count 4? I see one per type and one both have in common... \$\endgroup\$Timothy Truckle– Timothy Truckle2017年01月24日 15:16:22 +00:00Commented Jan 24, 2017 at 15:16
-
\$\begingroup\$ @SylvainBoisse "Nitpick 1: [...], Nitpick2: [...]" has been late yesterday, will fix it. \$\endgroup\$Timothy Truckle– Timothy Truckle2017年01月24日 15:17:24 +00:00Commented Jan 24, 2017 at 15:17
-
\$\begingroup\$ But if i need only min inclusive? And how to call them? CompareMode.inRange(.. ) or Cm..Mode.RANGE_INCLUSIVE.inRange(...) \$\endgroup\$Michael– Michael2017年01月24日 15:18:53 +00:00Commented Jan 24, 2017 at 15:18
checkMode
an enumeration? \$\endgroup\$