Skip to main content
Code Review

Return to Answer

added 421 characters in body
Source Link

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);
 }

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);
 }
Source Link

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.
lang-java

AltStyle によって変換されたページ (->オリジナル) /