Open-closed principle with constructor

Question

Learning 'SOLID' principle I'm wondering is it ok to modify a constructor if I need to add some more extension to class, eg. business logic.

From what I've learned it looks like modifying constructor I violate the 'open-closed' principle, but what if I need to inject another class to perform some logic? How can I do this without constructor modification, and in general, does constructor modification violate 'open-closed' principle?

Let's consider an example.

There's an interface

public interface ShopFactory {
    List<Discount> getDiscounts();
    List<Sale> getSales();
}

And there's one implementation (and possibly there could be others if somebody wants to add my library as a dependency)

public class CountableDefaultShopFactory implements ShopFactory {

    Counter discountsCounter;
    Counter salesCounter;

    public DefaultShopFactory(Counter discountsCounter, Counter salesCounter) {
        this.discountsCounter = discountsCounter;
        this.salesCounter = salesCounter;
    }

    @Override
    List<Discount> getDiscounts() {
        discountsCounter.count();
        return Discount.defaultDiscounts();
    }

    @Override
    List<Sale> getSales() {
        salesCounter.count();
        return Sale.defaultSales();
    }

}

So it's pretty straightforward. CountableDefaultShopFactory implements ShopFactory, overrides two methods and depends on some Counter objects in order to count how many times each method has been called. Each method returns some data using static method.

Now let's say that I've been asked to add one more method, and this time I need to fetch data from some storage and there's a service which provides some data from that storage. In this case, I need to inject this service in my class in order to perform an operation.

And it's going to looks like this:

public class CountableDefaultShopFactory implements ShopFactory {

    Counter discountsCounter;
    Counter salesCounter;
    Counter couponsCounter;
    CouponDAO couponDAO;

    public DefaultShopFactory(Counter discountsCounter, Counter salesCounter, Counter couponsCounter, CouponDAO couponDAO) {
        this.discountsCounter = discountsCounter;
        this.salesCounter = salesCounter;
        this.couponsCounter = couponsCounter;
        this.couponDAO = couponDAO;
    }

    @Override
    List<Discount> getDiscounts() {
        discountsCounter.count();
        return Discount.defaultDiscounts();
    }

    @Override
    List<Sale> getSales() {
        salesCounter.count();
        return Sale.defaultSales();
    }

    @Override
    List<Coupon> getCoupons() {
        couponsCounter.count();
        return couponDAO.getDefaultCoupons();
    }

}

So I had to modify my constructor by adding one more Counter class and
CouponDAO. I'm ok that I need to add one more Counter called couponsCounter since this is a countable implementation of ShopFactory. But adding CouponDAO doesn't look good for me.

I'm wondering is there a better solution how to do this?

Answer 1

Yes, it's violating Open Closed, but also SRP, as you've now given the class more than one reason to change.

A new requirement came along and you could have extended the code without changing what was there and adding all those new dependencies that were only used by a single method. (I'll drop the Factory postfix if you don't mind):

public interface CouponShop extends Shop {
    List<Coupon> getCoupons();
}

public class CountableCouponShop implements CouponShop {

    public CountableCouponShop(Shop shop, Counter couponsCounter, CouponDAO couponDAO) {
        //assign to fields
    }

    @Override
    List<Discount> getDiscounts() {
        return shop.getDiscounts(); //just delegate to the old implementation of shop
    }

    @Override
    List<Sale> getSales() {
        return shop.getSales();
    }

    @Override
    List<Coupon> getCoupons() {
        couponsCounter.count();
        return couponDAO.getDefaultCoupons();
    }
}