1
\$\begingroup\$

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

asked Jan 23, 2017 at 21:00
\$\endgroup\$
7
  • 1
    \$\begingroup\$ Why isn't checkMode an enumeration? \$\endgroup\$ Commented Jan 23, 2017 at 21:03
  • \$\begingroup\$ That is a very good question, @Alexander. I want to ask it too. Michael, why you aren't using enumerations? Oh, me? I did not thinked about that. Yes, we can get values ids, so it might be used the same way. But also it will protect the code and will help normal developres to use it. Thx for question! \$\endgroup\$ Commented Jan 23, 2017 at 21:06
  • \$\begingroup\$ You don't need to access the enum's raw values. You can use enum values directly in switches and equality comparison in if statements. \$\endgroup\$ Commented Jan 23, 2017 at 21:08
  • \$\begingroup\$ That is good way to. I need to tink what will be better. Switch will be easy to read, and code is small, so might be easy to check. Thx for pointing this :~) \$\endgroup\$ Commented Jan 23, 2017 at 21:10
  • \$\begingroup\$ It will be. The whole point of enums is to abstract away these irrelevant details about the backing representation of an enumerated choice. \$\endgroup\$ Commented Jan 23, 2017 at 21:12

2 Answers 2

1
\$\begingroup\$

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.

answered Jan 24, 2017 at 17:02
\$\endgroup\$
1
\$\begingroup\$

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);
 }
answered Jan 23, 2017 at 22:41
\$\endgroup\$
9
  • \$\begingroup\$ Excellent answer. Nitpick 1: you're dealing twice with should I throw, Nitpick2: it's called a Tuple. \$\endgroup\$ Commented Jan 24, 2017 at 14:12
  • \$\begingroup\$ So, it will be 4 methods for each check type. Also, how are you calling methods in enum? \$\endgroup\$ Commented 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\$ Commented Jan 24, 2017 at 15:16
  • \$\begingroup\$ @SylvainBoisse "Nitpick 1: [...], Nitpick2: [...]" has been late yesterday, will fix it. \$\endgroup\$ Commented 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\$ Commented Jan 24, 2017 at 15:18

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.