Reputation: 336
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
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
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