How to encapsulate private fields that only apply to a few methods

Question

I'm working on modeling a business domain object in a class and am wondering what would be the best way to properly encapsulate private fields that only apply to a few methods.

When I started, my code originally looked like this:

public class DiscountEngine
{
    public Cart As Cart { get; set;}
    public Discount As Discount { get; set;}

    public void ApplySKUGroupDiscountToCart()
    {
        ...
    }
}

However, ApplySKUGroupDiscountToCart() was starting to get ugly, so I decided to refactor the code into smaller private methods that get called from ApplySKUGroupDiscountToCart(). I started by passing in lots of local variables into the helper method, but then decided to pull out variables common to both routines and make them private modular variables. The new code looks like this:

public class DiscountEngine
{
    public Cart As Cart { get; set;}
    public Discount As Discount { get; set;}

    private int _SKUGroupItemDiscountsApplied = 0
    private int _SKUGroupTotalDiscounts = 0
    private int _SKUGroupID = 0

    public void ApplySKUGroupDiscountToCart()
    {
        ...
    }

    private void ApplyDiscountToSingleCartItem(ref CartItem cartI, 
                                               ref DiscountItem discountI)
    {
        ...
    }
}

On the one hand, the three private integer fields are useful for allowing the related methods to share common variables without needing to pass them back and forth as parameters. However, these variables are only applicable to these related methods and any other methods I might add would have no need to see them.

Is there a way to encapsulate the private fields and their related methods while still remaining a part of the DiscountEngine class? Is there a better way altogether of dealing with this problem?

Answer 1

I would say the only other way that I can think of to handle this would be to extract all the methods and private variables that are associated with them into a separate class. That way you keep all that encapsulated. But not sure if that would make sense in the context of your domain objects.

Answer 2

You could always create a nested (inner) class to bundle together parameters that have a common use. In this way you could still pass them to your private methods without having to pass around l.ots of arguments - you'd just pass an instance of the private type.

Answer 3

First things first, you very likely don't need to pass the parameters to ApplyDiscountToSingleCartItem as ref. Short version: unless you're actually assigning a value to the variable that you want to be visible to the calling code, you don't need ref. Modifying variable and property values on them will be visible to the calling code without passing them as ref.

Second, there is no way to scope a variable in between instance and local, which is what you're asking. The only way to accomplish this would be to refactor this functionality into another class (likely a nested private class).

Don't, however, use instance variables as a way to pass data between functions. If the data becomes "stale" after the function is called, then it should be a parameter, not an instance variable.

Answer 4

Normally, making a class field private implies "I have enough discipline to ensure that this field is only used in an appropriate manner inside this class". If your class is too big for you to say that with confidence, then maybe the class is trying to do too many different things, and should be split up (see SRP).

Anyway, enough of the theory :-). If you want to stick with one class then you could always encapsulate those three fields into a private nested class, e.g.

public class DiscountEngine
{
    public Cart As Cart { get; set;}
    public Discount As Discount { get; set;}

    private class SKUGroup
    {
        public int ItemDiscountsApplied = 0
        public int TotalDiscounts = 0
        public int ID = 0
    }

    public void ApplySKUGroupDiscountToCart()
    {
        ...
    }

    private void ApplyDiscountToSingleCartItem(ref CartItem cartI, 
                                               ref DiscountItem discountI)
    {
        ...
    }
}

That gives you a bit more freedom to pass instances of the class around your code as method parameters.

You could take this a step further, and move any private methods that act on the SKU data into the nested class as well.

Answer 5

"these variables are only applicable to these related methods and any other methods I might add would have no need to see them."

First of all, keep in mind that one of the first rules of OO development is to build what the customer wants THEN apply OO design like basic OO rules and patterns. Your quote verges on saying you want to plan for the unknown. Be careful that the unknown is "more of the same" not NEW requirements. Otherwise, this class is going to end up becoming a God Object.

If you find you have many members that aren't used by the methods, then divide and conquer.