c++chain-of-responsibility

Chain of responsibility going back by pointers for no reason

So I had a task to do a shop class, products class a basket class bla bla bla, I did all of this, tested, everything is working fine and that has nothing to do with the problem. Now what I had to do is create discounts and a chain of responsibility, like let's say I create 3 discounts and it goes through pointers and checks which one is suitable. So I did it, it does work, when I was debugging I checked, that it stops on the discount it needs to, returns me the price with the discount applied BUT THEN it somehow is going back by pointers and returning again what I don't need. I will make this more clear after I show the code and I will provide an example. So this is my Discount.h which holds a pointer to the next Discount and the main virtual countDiscount(); function that plays the role in all of this.

#ifndef DISCOUNT_HEADER
#define DISCOUNT_HEADER

#include "Basket.h"

class Discount
{
public:
    Discount():next(NULL){}
    virtual int countDiscount(Basket b) = 0;
    void nextDiscount(Discount* nextDisc) { next = nextDisc; }
protected:
    Discount* next;
};

#endif

So, then I had to do a lot of discount types so I started with a fixated discount, let's say I have FixatedDiscount(100, 15), so when the person's basket's contents are worth more than 100, then the discount is applied and it just takes 15 away from his total value. So if he shopped for like 110 then the discount applied and it did 110 - 15= 95. This is the FixatedDiscount.h

#ifndef FIXATED_HEADER
#define FIXATED_HEADER

#include "Discount.h"

class FixatedDiscount:public Discount
{
public:
    FixatedDiscount(int l, int d) : limit{ l }, discount{ d } {}
    int countDiscount(Basket b);
private:
    int limit;
    int discount;
};

#endif

And this is FixatedDiscount.cpp

#include "FixatedDiscount.h"
#include <iostream>

int FixatedDiscount::countDiscount(Basket b) {
    int totalPrice = b.countPrice(); //this method just counts the value of the basket which for our example is 120
    if (totalPrice >= limit) {
        totalPrice -= discount;
        std::cout << "Handled when limit is " << limit << " and discount is: " << discount << " returning total price: " << totalPrice << std::endl;
    }
    else if (next != NULL) {
        next->countDiscount(b);
    }
    else {
        std::cout << "I am the last handler" << std::endl;
    }
    return totalPrice;
}

So let's say I had 3 discounts, FixatedDiscount(150, 20);, FixatedDiscount(100, 15), FixatedDiscount(50, 10). In my example I will make a basket that's worth 120, so it should skip the first discount and move on to the second one since he shopped more than 100 and the discount of 15 is applied but he didn't reach 150 to get discount of 20.

#include <iostream>
#include "Product.h"
#include "Basket.h"
#include "Discount.h"
#include "FixatedDiscount.h"
#include <vector>

int main()
{
    Product* p1 = new Product("banana", 20, "fruit");
    Product* p2 = new Product("coconut", 35, "fruit");
    Product* p3 = new Product("watermelon", 45, "vegetable");
    Product* p4 = new Product("cucumber", 20, "vegetable");
    Product* p5 = new Product("butter", 30, "dairy");
    Product* p6 = new Product("milk", 40, "dairy");
    std::vector<Product*> catalog{ p1, p2, p3, p4, p5, p6 };

    Basket basket;
    basket.insert(catalog[1]);
    basket.insert(catalog[2]);
    basket.insert(catalog[5]); //All of these 3 products make up to value of 120

    Discount* fixated1 = new FixatedDiscount(150, 20);
    Discount* fixated2 = new FixatedDiscount(100, 15);
    Discount* fixated3 = new FixatedDiscount(50, 10); //made these 3 discounts
    fixated1->nextDiscount(fixated2);
    fixated2->nextDiscount(fixated3); //linked by pointers these 3 discounts
    std::cout << "Price with the discount: " << fixated1->countDiscount(basket) << std::endl;
}

So what is going here is that as you saw in FixatedDiscount.cpp I made a simple cout to check which discount was reached, and it says Handled when limit is 100 and discount is: 15, returning total price: 105 as it should be! BUT THEN, it somehow goes through the countDiscount function once again and returns 120, which is the price without the discount applied, how is this possible? It does not go to the third discount, but somehow it goes through that function once more and returns 120, so my theory is that when it reached the second discount it returned me what I want which is 105 since a discount of 15 was applied, then it goes back to the first pointer which is the first discount, and since 120 is not more than 150 it does not apply the discount and returns me 120. Why doesn't it just return me what I want and stop? Why does it go through that function once more possibly going back by the pointer and applying again the discount that was already checked and shouldn't be checked anymore.

Solution 
  • int FixatedDiscount::countDiscount(Basket b) {
    int totalPrice = b.countPrice(); //this method just counts the value of the basket which for our example is 120
    if (totalPrice >= limit) {
        totalPrice -= discount;
        std::cout << "Handled when limit is " << limit << " and discount is: " << discount << " returning total price: " << totalPrice << std::endl;
    }
    else if (next != NULL) {
        next->countDiscount(b);
    }
    else {
        std::cout << "I am the last handler" << std::endl;
    }
    return totalPrice;
}

    After calling the next->countDiscount(b);, the function doesn't return, it returns totalPrice which is unmodified after the discount is applied. You should assign the return value here for that discount to take effect.

    totalPrice = next->countDiscount(b); should return 105.