Let's look for some patterns in your code:
First of all, we notice that you have three parameters with similar names. condition
, condition2
and condition3
. These names indicates a code smell, that we should use an array or list instead.
Taking one step at a time, we see this code is repeated multiple times:
if (StringUtils.isNotEmpty(condition.toString())) {
query.append(ApplicationConstants.AND+"(");
query.append("( " + condition + " )");
if (StringUtils.isNotEmpty(condition2.toString())) {
query.append(ApplicationConstants.AND);
query.append("( " + condition2 + " )");
if (StringUtils.isNotEmpty(condition3.toString())) {
query.append(ApplicationConstants.AND);
query.append("( " + condition3 + " )");
if (StringUtils.isNotEmpty(condition3.toString())) {
query.append(ApplicationConstants.AND);
query.append("( " + condition3 + " )");
if (StringUtils.isNotEmpty(condition2.toString())) {
query.append(ApplicationConstants.AND+"(");
query.append("( " + condition2 + " )");
if (StringUtils.isNotEmpty(condition3.toString())) {
query.append(ApplicationConstants.AND);
query.append("( " + condition3 + " )");
if (StringUtils.isNotEmpty(condition3.toString())) {
query.append(ApplicationConstants.AND+"(");
query.append("( " + condition3 + " )");
This indicates that we can extract a method.
private boolean addParameter(StringBuilder query, StringBuilder condition) {
if (StringUtils.isNotEmpty(condition.toString())) {
query.append(ApplicationConstants.AND+"(");
query.append("( " + condition + " )");
return true;
}
return false;
}
private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
StringBuilder condition3) {
if (addParameter(condition)) {
if (addParameter(condition2)) {
if (addParameter(condition3)) {
// we don't actually care about the result here
}
} else {
if (addParameter(condition3)) {
// not here either
}
}
query.append(" )");
} else {
if (addParameter(condition2)) {
if (addParameter(condition3)) {
// again, don't care about result
}
query.append(" )");
} else {
if (addParameter(condition3)) {
// yeah ok, here we actually care...
query.append(" )");
}
}
}
Now it's pretty clear that query.append(" )");
should always be done last, so that can be moved out and then we can avoid some if
statements.
private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
StringBuilder condition3) {
if (addParameter(condition)) {
if (addParameter(condition2)) {
addParameter(condition3);
} else {
addParameter(condition3);
}
} else {
if (addParameter(condition2)) {
addParameter(condition3);
} else {
addParameter(condition3);
}
}
query.append(" )");
And now we're doing the same in both some if's and else's, so that can be simplified:
private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
StringBuilder condition3) {
if (addParameter(condition)) {
addParameter(condition2);
addParameter(condition3);
} else {
addParameter(condition2);
addParameter(condition3);
}
query.append(" )");
Once again we're doing the same thing:
private void appendConditions(StringBuilder query, StringBuilder condition, StringBuilder condition2,
StringBuilder condition3) {
addParameter(condition);
addParameter(condition2);
addParameter(condition3);
query.append(" )");
So we're essentially doing the same thing three times. This is where a loop would be handy.
private void appendConditions(StringBuilder query, StringBuilder... conditions) {
for (StringBuilder condition : conditions) {
addParameter(condition);
}
query.append(" )");
And now of course, we don't really need that separate addParameter
method.
private void appendConditions(StringBuilder query, StringBuilder... conditions) {
for (StringBuilder condition : conditions) {
if (StringUtils.isNotEmpty(condition.toString())) {
query.append(ApplicationConstants.AND+"(");
query.append("( " + condition + " )");
}
}
query.append(" )");
Voilà!
- 59.7k
- 9
- 157
- 311