Which one is better in terms of readability? [closed]

Closed. This question is off-topic. It is not currently accepting answers.

---

Want to improve this question? Update the question so it's on-topic for Stack Overflow.

Closed 11 years ago.

Improve this question

If you have to check 3 conditions, and in an cases return the same value, like in this example:

        if (designatorType != AttributeDesignator.ENVIRONMENT_TARGET)
            return new EvaluationResult(BagAttribute.
                                        createEmptyBag(attributeType));

        if (! attributeId.toString().equals("processor-load"))
            return new EvaluationResult(BagAttribute.
                                        createEmptyBag(attributeType));

        if (! attributeType.toString().equals(IntegerAttribute.identifier))
            return new EvaluationResult(BagAttribute.
                                        createEmptyBag(attributeType));

Wouldn't it be better to rewrite it like this

        if (designatorType != AttributeDesignator.ENVIRONMENT_TARGET) ||
           (! attributeId.toString().equals("processor-load")) ||
           (! attributeType.toString().equals(IntegerAttribute.identifier)) ||
            return new EvaluationResult(BagAttribute.
                           开发者_高级运维             createEmptyBag(attributeType));

This is probably highly subjective but still I hope someone can give a valid answer :-).

---

public void someMethod()
{
  if (isConditionMet())
    return new EvaluationResult(BagAttribute.createEmptyBag(attributeType));
}

private boolean isConditionMet()
{
  if (designatorType != AttributeDesignator.ENVIRONMENT_TARGET))
    return true;
  if (! attributeId.toString().equals("processor-load"))
    return true;
  if (! attributeType.toString().equals(IntegerAttribute.identifier))
    return true;
  return false;  
}

---

No confusing at all. You have three independent preconditions that lead to the same postcondition (result).

---

I would create a separate method for the condition.

private boolean shouldCreateEmptyBag(...){

   return (designatorType != AttributeDesignator.ENVIRONMENT_TARGET) ||
           (! attributeId.toString().equals("processor-load")) ||
           (! attributeType.toString().equals(IntegerAttribute.identifier)) || ...
}

then:

if(shouldCreateEmptyBag(...)){
    return new EvaluationResult(BagAttribute.
                                        createEmptyBag(attributeType));
}

---

I prefer the first one - I've seen too many people getting their operators confused when there are multiples of them in one statement.

---

if (isTest1(designatorType) || isTest2(attributeId) || isTest3(attributeType)){
    return new EvaluationResult(BagAttribute.createEmptyBag(attributeType));
}

---

Both of these code are same. First one is more clear and separated by the conditions which is independent with each other. But the second one is only the combination of these conditions. IMO the first one is more flexible and understandable. If you made any change in future it will be easier to understand where to change and what to change.