0

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!

Improve this question
2
  • Touche. Any idea how it can improved. :) Commented May 23, 2020 at 5:54
  • Some applications need such if statements. I programmed an early web-application that had similar filtering, and I used a boat-load of if statements. Commented May 23, 2020 at 6:24

2 Answers 2

Reset to default
2

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`
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

Thanks @lucid. I know it will work, have not tried it yet with this setup of code as yet. But thanks anyways.
Can you please explain how this piece of code is working
Thanks again, this worked like a charm, without any overhead. Great!
fields.entrySet() will basically returns all fields, and stream will help iterate over it. filter is used to filter out fields that doesn't match criteria. filter.get(entry.getKey()) returns filter for field (entry.getKey()) and .apply will apply that filter on value (entry.getValue). if it fails, it will not move forward in the chain, otherwise, that entry will go to fieldQueryMapping and there we will get a respective query. collectors.joining("and") will join all entries which reached till this state with and.
I hope this helps you understand, @Sam
|
1

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.

Improve this answer

17 Comments

I think it will work, the problem I am facing is if the user applies the filter multiple times then that many time that part is added in the query string. See this example when I tried and ran my code. In next comment " and task_status_id in (1, 2, 3)" is added x number of times as filter is applied. I tried using regex to remove it, but failed to create a regex for that see here "replace("(?s)(\\and task_status_id in .*?\\/)\\\)", "")". It did not worked. Any idea how this can be resolved or is there a better approach to handle it. @PatrickChen
select * from view_daily_task where task_id in (select distinct(task_id) from map_task_person where receiver_id = ? or sender_id = ? and status=and task_status_id in (1, 2, 3) and task_status_id in (1, 2, 3, 4)1) order by latest_child_date_time desc
Obviously you have a bug in your if statements. You should try to re-organize the code reduces the redundant if statements, then debug the problem.
Simplify the code will help you improve readability and easier to debug.
I tried your idea and this statement query.append(" and ").append(taskIdQuery); adds the taskIdQuery part multiple times
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.