John
John

Reputation: 336

How to reduce if condition's?

I am developing a module in which there are 10 filters, on which a user can filter data shown in grid. Lets assume data is of team in a company in which there are 10 members and each person is given a task to do work. All the members of the team add those task in that module and submit it to their superiors telling them that they have done this work, on this date, in that much of time etc.

And their superiors can have a look at these submissions as number of task is more in number they need few filter conditions to filter out data. For example, they might want to see only x-person task or what he has done, or in last 2 days, what task are submitted to them by person-y etc.

All the filters are optional and none is applied when the page load and displays data first time.

As the user add filters, the data gets filtered. But here is the problem - Managing these filters and adding new filters as requirement change has been very trouble some for me. Due to large number of IF Statements.

At present my code something like this

if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime == null || lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and " + taskStatusQuery;
            }
            if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and " + lastModified + " and " + taskStatusQuery;
            }
            if ((strIdList == null || strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and " + lastModified + " and " + taskStatusQuery;
            }
            if (strTaskDate != null && !strTaskDate.isEmpty() && (strIdList == null || strIdList.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty())) {
                query = query + " and " + taskDateQuery + " and " + taskStatusQuery;
            }
            if (strWorkDate != null && !strWorkDate.isEmpty() && (strTaskDate == null || strTaskDate.isEmpty()) && (strIdList == null || strIdList.isEmpty())) {
                query = query + " and " + workDateQuery + " and " + taskStatusQuery;
            }
            if (strWorkDate != null && !strWorkDate.isEmpty() && strTaskDate != null && !strTaskDate.isEmpty() && strIdList != null && !strIdList.isEmpty()) {
                query = query + " and " + taskIdQuery + " and " + taskDateQuery + " and " + workDateQuery + " and " + taskStatusQuery;
            }
            if (strTaskDate != null && !strTaskDate.isEmpty() && strWorkDate != null && !strWorkDate.isEmpty() && (strIdList == null || strIdList.isEmpty())) {
                query = query + " and " + taskDateQuery + " and " + workDateQuery + " and " + taskStatusQuery;
            }
            if (strTaskDate != null && !strTaskDate.isEmpty() && (strWorkDate == null || strWorkDate.isEmpty()) && strIdList != null && !strIdList.isEmpty()) {
                query = query + " and " + taskIdQuery + " and " + taskDateQuery + " and " + taskStatusQuery;
            }
            if ((strTaskDate == null || strTaskDate.isEmpty()) && strWorkDate != null && !strWorkDate.isEmpty() && strIdList != null && !strIdList.isEmpty()) {
                query = query + " and " + taskIdQuery + " and " + workDateQuery + " and " + taskStatusQuery;
            }
            if ((strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (strIdList == null || strIdList.isEmpty()) && (lastModifiedDateTime == null || lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskStatusQuery;
            }
        } else if (filterParameter == 2) {

            query = query + " and " + noWorkPerformedQuery;

            if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (lastModifiedDateTime == null || lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and " + taskStatusQuery;
            }
            if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and" + lastModified + " and " + taskStatusQuery;
            }
            if ((strIdList == null || strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and" + lastModified + " and " + taskStatusQuery;
            }
            if ((strIdList == null || strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (lastModifiedDateTime == null || lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskStatusQuery;
            }
            if (strTaskDate != null && !strTaskDate.isEmpty() && (strIdList == null || strIdList.isEmpty())) {
                query = query + " and " + taskDateQuery + " and " + taskStatusQuery;

            }
            if (strTaskDate != null && !strTaskDate.isEmpty() && strIdList != null && !strIdList.isEmpty()) {
                query = query + " and " + taskIdQuery + " and " + taskDateQuery + " and " + taskStatusQuery;

            }

And there's more of that... I know this very bad, but don't know how to improve this code. Thought of using switch but it does't help that much.

Why it has so many conditions because for example. Lets say I have two optional filters on name and date, then scenarios

Name    Date
True    True
True    False
False   True
False   False

Now you can understand where I am going. So as the number of filters add up these conditions multiple simple maths.

I hope someone can advice me how to handle such scenarios thanks!

Upvotes: 0

Views: 175

Answers (2)

lucid
lucid

Reputation: 2900

If you analyze your query conditions, they correspond to some query part. For example,

`strIdList != null && !strIdList.isEmpty()`  => taskIdQuery
`strTaskDate == null || strTaskDate.isEmpty()` => taskStatusQuery

Now, let's create a mapping for field and values

Map<String, String> fields = new HashMap<>();
fields.put("id", strIdList);
fields.put("strTaskDate", strTaskDate);

And let's initialize filter conditions, (condition on which you want to add query)

Map<String, Function<String, Boolean>> filters = new HashMap<>();
filters.put("id", StringUtils::isNotBlank);
filters.put("strTaskDate", StringUtils::isBlank);

//Can be written like this as well if you are not familiar with above

// filters.put("id", value -> value != null && !value.isEmpty());
// filters.put("strTaskDate", value -> value ==null || value.isEmpty());

And create a mapping for field and corresponding query (if condition satisfied)

Map<String, String> fieldQueryMapping = new HashMap<>();
fieldQueryMapping.put("id", "taskIdQuery");
fieldQueryMapping.put("strTaskDate", "taskStatusQuery");

fieldQueryMapping and filters would be one time efforts, can be initialized once.

Now, let's generate query

String gneratedQuery = fields.entrySet().stream()
  .filter(entry -> filters.get(entry.getKey()).apply(entry.getValue()))
  .map(entry -> fieldQueryMapping.get(entry.getKey()))
  .collect(Collectors.joining(" and "));

In the above-mentioned cases, if strIdList has value, and strTaskDate doesn't have value, it will generate following.

Map<String, String> fields = new HashMap<>();
fields.put("id", "strIdListValue");
fields.put("strTaskDate", "");

//output would be 
`taskStatusQuery and taskIdQuery`

Upvotes: 2

PatrickChen
PatrickChen

Reputation: 1430

The code is too hard to read. I look at it for 1 minute, I think something obviously can change. for example:

if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime == null || lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and " + taskStatusQuery;
}
if ((strIdList != null && !strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and " + taskIdQuery + " and " + lastModified + " and " + taskStatusQuery;
}
 if ((strIdList == null || strIdList.isEmpty()) && (strTaskDate == null || strTaskDate.isEmpty()) && (strWorkDate == null || strWorkDate.isEmpty()) && (lastModifiedDateTime != null && !lastModifiedDateTime.isEmpty())) {
                query = query + " and " + lastModified + " and " + taskStatusQuery;
}

This can be changed to:

//define a method to check string empty or using StringUtils in many libs 
public boolean isNotEmpty(String s) { return s != null && !s.isEmpty();}
//using StringBuilder in stead of +
StringBuilder q = new StringBuilder(query);

if (isNotEmpty(strIdList)) {
  query.append(" and ").append(taskIdQuery);
}
if (isNotEmpty(strTaskDate)) {
query.append(" and ").append(lastModified);
}
if (!isNotEmpty(strTaskDate) && !isNoteEmpty(strWorkDate)) {
query.append(" and ").append(taskStatusQuery);
}

I hope I'm not wrong, too much similar conditions.

Hopes it helps you.

Upvotes: 1

Related Questions