If Else - Repeated Code Logic

Question

My boss gave me a project with a particular logic. I have to develop a web page which has to lead the navigator through many cases until he/she arrives at the product.

This is the path scheme of the navigation in the site:

IMPORTANT!

In the Products page the navigator can choose which filter he wants.

• If A, he/she MUST go through the B (and then C of course) or C and reach the products.
• If B, he/she MUST go through the C and reach the products.
• If C, he/she reaches directly the products.

Of course if I start from A I am following the longest path and when I reach my products I have 3 active filters.

Until now I developed the following code which works fine.

if filter_A
  if filter_B
     filter_C()
     .. else ..
  else
     filter_C
    .. else ..
else
   if filter_B
      filter_C()
     .. else ..
   else
     filter_C()
     .. else ..

I'm here to ask what would a more expert programmer have done in this situation. I didn't respect the DRY principle, I don't like it and I'd like to know an alternative way to develop this kind of logic.

I thought about splitting every section of code in functions but is it a good idea in this case?

Answer 1

You haven't said whether the filters take any parameters. For example, filter_A might be a category filter, so that it's not just a question of "do I need to apply filter_A", it could be "I need to apply filter_A and return all records in with the category field = fooCategory".

The simplest way to implement exactly what you've described (but make sure to read the second half of the answer below) is similar to the other answers, but I wouldn't have any boolean checks at all. I would define interfaces: FilterA, FilterB, FilterC. Then you can have something like (I'm a Java programmer, so this will be Java-esque syntax):

class RequestFilters {
    FilterA filterA;
    FilterB filterB;
    FilterC filterC;
}

Then you could have something like this (using the enum singleton pattern from Effective Java):

enum NoOpFilterA implements FilterA {
    INSTANCE;

    public List<Item> applyFilter(List<Item> input) {
       return input;
    }
}

But if you actually want some items to be filtered, you can instead provide an instance of a FilterA implementation that actually does something. Your filtration method will be the very simple

List<Item> filterItems(List<Item> data, RequestFilters filters) {
    List<Item> returnedList = data;
    returnedList = filters.filterA.filter(data);
    returnedList = filters.filterB.filter(data);
    returnedList = filters.filterC.filter(data);
    return returnedList;
}

---

But I'm just getting started.

I suspect that the applyFilter call will actually be quite similar for all three types of filters. If that's the case, I wouldn't even do it the way described above. You can get even cleaner code by only having one interface, then doing this:

class ChainedFilter implements Filter {
     List<Filter> filterList;

     void addFilter(Filter filter) {
          filterList.add(filter);
     }

     List<Item> applyFilter(List<Item> input) {
         List<Item> returnedList = input;
         for(Filter f : filterList) {
             returnedList = f.applyFilter(returnedList);
         }
         return returnedList;
     }
}

Then, as your user navigates through the pages, you just add a new instance of whatever filter you need when appropriate. This will allow you to be able to apply multiple instances of the same filter with different arguments should you need that behavior in the future, and also add additional filters in the future without having to change your design.

Additionally, you can add either something like the NoOpFilter above or you can just not add a particular filter at all to the list, whatever is easier for your code.

Answer 2

In this case, it is important to separate the logic of filtering, and the control flow of how the filters run. The filter logic should be separated out into individual functions, that can run independent of each other.

ApplyFilterA();
ApplyFilterB();
ApplyFilterC();

In the sample code posted, there's 3 booleans filter_A, filter_B, and filter_C. However, from the diagram, filter_C always runs, so that can be changed to an unconditional.

NOTE: I am assuming that the control flow diagram is correct. There is a discrepancy between the posted sample code and the control flow diagram.

A separate piece of code controls which filters get run

ApplyFilters(bool filter_A, bool filter_B)
{
    listOfProducts tmp;
    if (filter_A)
        ApplyFilterA();
    if (filter_B)
        ApplyFilterB();
    ApplyFilterC();
}

There is a distinct separation between controlling which filters run, and what the filters do. Break those two pieces of logic apart.

Answer 3

I assume, that you want the simplest, clearest algorithm.
In this case, knowing that filter c is always applied, I would live it out of the if logic and apply it at the end regardless. As it looks in your flowchart, each filter before the c, is optional, because each of them can either be applied, or not. In this case, I would live ifs separate from each filter, without nesting and chaining:

if filter_a
  do_filter_a()

if filter_b
  do_filter_b()

do_filter_c()

if you have a flowchart with a variable number of filters, before the mandatory one, I would, instead, save all the filters to an array, in an order that they should appear. Then process optional filters in the loop and apply the mandatory one at the end, outside of the loop:

optional_filters_array = (a, b, c, d, e, f, g, h, etc)

for current_filter in optional_filters_array
  do_filter(current_filter)

do_required_filter()

or:

optional_filters_array = (a, b, c, d, e, f, g, h, etc)
required_filter = last_filter


for current_filter in optional_filters_array
  do_filter(current_filter)

do_filter(required_filter)

of cource, you would have to define the filter processing subroutine.

Answer 4

I'm going to assume filterA, filterB, and filterC actually modify the list of products. Otherwise, if they are just if-checks, then filterA and filterB can be ignored since all paths lead ultimately to filterC. Your description of the requirement seems to imply that each filter will reduce product list.

So assuming the filters actually reduce the list of products, here's bit of pseudo-code...

class filter
    func check(item) returns boolean
endclass

func applyFilter(filter, productList) returns list
    newList is list
    foreach item in productList
        if filter.check(item) then
            add item to newList
        endif
    endfor 
    return newList
endfunc



filterA, filterB, filterC = subclasses of filter for each condition, chosen by the user
products = list of items to be filtered

if filterA then
    products = applyFilter(filterA, products)
endif

if filterB then
    products = applyFilter(filterB, products)
endif

if filterC then
    products = applyFilter(filterC, products)
endif

# use products...

In your requirements, filterC is not automatically applied, but in the diagram, it is. If the requirement is that at least filterC should be applied no matter what, then you would call applyFilter(filterC, products) without checking if filterC is chosen.

filterC = instance of filter, always chosen

...

# if filterC then
products = applyFilter(filterC, products)
# endif

Answer 5

I wonder if modeling your filters to be some kind of objects in a graph would make sense. At least that's what I think of when seeing the diagram.

If you model the dependency of the filters like a object graph, then the code that handles the possible flow paths is pretty much straight forward without any hairy logic. Also, the graph (business logic) can change, while the code that interprets the graph stays the same.