Class that represents a conditional expression

Question

I have a class with which I want to persist a conditional expression:

boolean x = a [GT] i

I am only concerned with the boolean outcome of that comparison. 'a' '[GT]' and 'i' are supplied as arguments at runtime.

public class RuleClauseCondition {
 RuleClauseOperator operator;
 Double threshold;
 Double operand;
 private RuleClauseCondition()
 {
 }
 public RuleClauseCondition(RuleClauseOperator operator, Double threshold, Double operand) {
 this.operator = operator;
 this.threshold = threshold;
 this.operand = operand;
 }
 boolean isValid() {
 return operator.isValid(operand, threshold);
 }
}
public enum RuleClauseOperator{
 EQ {
 @Override
 public boolean isValid(Double inputValue, Double thresholdValue) {
 return inputValue.equals(thresholdValue);
 }
 @Override
 public String toString() {
 return " == ";
 }
 }, LT {
 @Override
 public boolean isValid(Double inputValue, Double thresholdValue) {
 return inputValue < thresholdValue;
 }
 @Override
 public String toString() {
 return " < ";
 }
 }, GT {
 @Override
 public boolean isValid(Double inputValue, Double thresholdValue) {
 return inputValue > thresholdValue;
 }
 @Override
 public String toString() {
 return " > ";
 }
 }

I am looking for better names to represent the left and right side of the operator as well as the class name.

Are current names intuitive and self documenting, or do I need to improve them?

• 200_success: Which version of Java are you writing for?
• Reg Mem: @200_success Java 8+, but I don't understand the importance of Java version here
• h.j.k.: @RegMem in that case you have BiPredicate from Java 8...
• andrejb-dev: be carefull and stable with the order in witch you use the threshold and operand, in comparison it is important. In the constructor the first (=left side) is threshold, in the clause operator the threshold is on the right side. Not everyone is good in math naming, so even if it can be correct, its confusing - i would include the "left" and "right" words in the names of double variables, i.e. leftSide or leftOperand...

Answer 1

Well, being the class a binary comparator, and being that the enum contains comparison operators, I would suggest to call them BinaryComparator and ComparisonOperator. Unless you have a naming convention or something else that would make those names not usable, obviously.

I would suggest naming classes by what they represent, if possible it should be a name not related to the context (if such names don't make the code less readable or less understandable).

• nhgrif: This seems like a comment and not an answer. There are no sentences in this post--only questions.
• Gentian Kasa: Thinking about it, you are right. I'm gonna rephrase the whole thing.
• Legato: Sensitive to the fact you couldn't comment, even if you wanted to. +1. Welcome to CodeReview.
• Gentian Kasa: Thanks mate, really appreciate it. But @nhgrif was right and I could have phrased the whole thing as a response (the nerd in me just doesn't yet care enough of the form as much as the content). Gotta pay more attention to that :)

Answer 2

@200_success Java 8+, but I don't understand the importance of Java version here

As mentioned in my comment, you have the use of BiPredicate here, so I'll suggest implementing that for your enum such that you can leverage on the standard JDK feature set (showing only the over-ridden method below).

enum MyPredicates implements BiPredicate<Double, Double> {
 EQ {
 @Override
 public boolean test(Double t, Double u) {
 return t.equals(u);
 }
 },
 LT {
 @Override
 public boolean test(Double t, Double u) {
 return t.compareTo(u) < 0;
 }
 },
 GT {
 @Override
 public boolean test(Double t, Double u) {
 return t.compareTo(u) > 0;
 }
 };
}

One small change you might notice is that I rely on compareTo() instead of an implicit unboxing using arithmetic operators. Ideally, you'll want to check against null values too.

Using it will be:

boolean result = MyPredicates.LT.test(0.4, 0.5); // true

Now I'm not so sure why you need a RuleClauseCondition as well, since such comparison operations are usually once-off and you simply save the result into a boolean value if you want to use that repetitively. Hence, I'll even suggest removing that to simplify your code.

edit

BTW, once you declare a constructor with arguments, there wouldn't be the default no-args constructor, so you wouldn't need private RuleClauseCondition() { }.

Also, after a re-read of your question, I suppose you wanted RuleClauseCondition to generate a toString() representation of the comparison? If so, I guess this makes sense, but I'll suggest just storing the result and the String representation immediately as such (now assume the enums' toString() methods are over-ridden in the way you described):

public class RuleClauseCondition {
 private final boolean result;
 private final String toString;
 public RuleClauseCondition(MyPredicates operator, Double threshold, Double operand) {
 Stream.of(operator, threshold, operand).forEach(Objects::requiresNonNull);
 result = operator.test(operand, threshold);
 toString = String.join(" ", operand.toString(), 
 operator.toString(), threshold.toString());
 }
 public boolean isValid() {
 return result;
 }
 @Override
 public String toString() {
 return toString;
 }
}

Answer 3

Are current names intuitive and self documenting

Yes and no. For me EQ, LT, and GT are perfectly clean, but other may want something more verbose. But I'm having a problem with RuleClauseOperator:

• Rule? Maybe in a context I don't know.
• Clause? OK, everything is clause, depending on what you mean.
• Operator? That's better, but you call it as EQ.isValid(1, 2), so it's just a binary function.

In your context the name may be alright, but standing alone it should be named more according to what it does. Maybe BinaryDoubleComparison? Naming is hard, indeed.

I dislike isValid. While 1 LT 2 is true, it may mean that something is invalid. An evaluation should not be that emotional. I'd go simply for apply just like Guava's Predicate. This name may sound strange, but it's the same as used for Function and Predicate is just a special case of it (though not a subinterface, though in a better world it would be).

• Reg Mem: Yes Rule is context specific, because the problem domain it addresses is a set of rules which are parsed against given values. I do like the apply() but it does not justify or go well with the boolean return type. Another option I could think of is isThresholdBreached instead of isValid
• maaartinus: @RegMem Standing alone, there's no threshold, just two inputs to compare (I'd called them left and right, or alike). Moreover, in case of EQ there's no threshold at all.
• Simon Forsberg: @RegMem isThresholdBreached sounds like the opposite of isValid. For the record, in Java 8 the method name for Predicate is test

Answer 4

I think the enums and methods are fine, I would probably append Validator or Check to the name of the problem domain. Something like InSeriesValidator or maybe even PipelineCheck for example.

Answer 5

I am only concerned with boolean outcome of that comparision, 'a' '[GT]' and 'i' are supplied as arguements at runtime.

Below is the class I have created:

public class RuleClauseCondition {
 RuleClauseOperator operator;
 Double threshold;
 Double operand;

You can just persist a boolean, and call it conditionHolds, then.

• Reg Mem: Cannot hold the result since the outcome is evaluated at runtime
• abuzittin gillifirca: Run-time means, not known at compile-time. Anything not constant is evaluated at runtime. What you cannot differentiate is persistent vs transient. If you mean, by saying 'a' '[GT]' and 'i' are supplied as arguements at runtime, that all of them are transient, why persist anything? What do you currently and actually persisting in the DB table.
• Reg Mem: So check this statement out if [student score] [>=] [90] then [do x()] So here [student score] is extracted from db by calculating few averages, [>=] is only known from DB configured value, it can be [<= or == or <] from db persisted value and [90] is also retrieved from persisted db value. The class that evaluates the result of final construct does not know the value of any parameters at compile time. That's what I meant by runtime