Is it ever a good idea to have classes intermixed/reliant on helper classes? C#

Question

I have a simple class that has a private field and a public property. The property also calls a static helper method which it is kinda dependant on and i feel this might not be a good thing.

This is my code:

public class PathLine
{
    private Vector2Int _gridA;
    public Vector2Int GridA
    {
        get { return _gridA; }
        set
        {
            _gridA = value;
            PointAInWorld = Grids.ToWorld(gridA, 1, Grids.GridPlane.XZ);
        }
    }

    public Vector3 PointAInWorld { private set; get; }
    private Vector2Int _gridB;
    public Vector2Int GridB
    {
        get { return _gridB; }
        set
        {
            _gridB = value;
            PointBInWorld = Grids.ToWorld(_gridB, 1, Grids.GridPlane.XZ);
        }
    }

    public Vector3 PointBInWorld { private set; get; }
}

Is this generally bad because of its dependency ? Is there a cleaner way of doing this so the class is less dependant?

Answer 1

Is this generally bad because of its dependency ?

There is no inherent bad practice rule that has been violated here. If anything, dependencies are the first step in the right direction (when you start from a single monolithic method).

There is no real issue here, provided that this static method fits the criteria for being a static method. THe main criteria being:

• The static method does not rely on an internal state, and only relies on the method parameters that you provide it with.
• The operation represented by the static method is globally correct and not context-specific.

That seems to be the case for your code, so there is no issue.

Is there a cleaner way of doing this so the class is less dependant?

Not as a global rule without any particular given context. In certain cases, it may be warranted to use an interface and dependency injection, but that's not the case for your code. I'll elaborate on the differences.

When to use static methods:

Based on your static method, I infer that the method is universally correct, and that there will never be a "competitor method" that is different from the original method but functionally equivalent.

For example:

public static int AddNumbers(int a, int b)
{
    return a + b;
} 

This is universally correct, and therefore should be a static method.

When to use dependency injection:

Suppose you're relying on an external source (e.g. Google) for doing the calculation:

public static int AddNumbers(int a, int b)
{
    return Google.Query($"{a} + {b}");
} 

Initially, you might make this a static method as you intend for this to be your only external resource. However, you should identify that it's possible for Google to be offline, or maybe even discontinue their service of doing mathematical calculations.

You want your application to be able to query other resources when you can no longer rely on Google. So let's say you use WolframAlpha as a backup. Because you now have two competing but functionally equivalent options, you can't make them static methods.

Well, you could:

public static int AddNumbersViaGoogle(int a, int b)
{
    return Google.Query($"{a} + {b}");
} 

public static int AddNumbersViaWolframAlpha(int a, int b)
{
    return WolframAlpha.Add(a, b);
} 

...but it would be bad practice.

Instead, you create an interface, and then inject the correct dependency into your class:

public interface INumberAddition
{
    int Add(int a, int b);
}

The interface stipulates the contract that both the Google and WolframAlpha handlers need to follow.

public class GoogleAdder : INumberAddition
{
    public int Add(int a, int b)
    {
        return Google.Query($"{a} + {b}");
    } 
}

public class WolframAlphaAdder : INumberAddition
{
    public int Add(int a, int b)
    {
        return WolframAlpha.Add(a, b);
    } 
}

You then change your class to receive any object which implements INumberAddition:

public class Calculator
{
    private readonly INumberAddition _additionHandler;

    public Calculator(INumberAddition additionHandler)
    {
        _additionHandler = additionHandler;
    }

    public int FindOutWhatOnePlusOneIs()
    {
         return _additionHandler.Add(1,1);
    }
}

This way, you can choose your dependency on a higher level:

var googleCalculator = new Calculator(new GoogleAdder());
var googleResult = googleCalculator.FindOutWhatOnePlusOneIs();

var wolframCalculator = new Calculator(new WolfranAlphaAdder());
var wolframResult = wolframCalculator.FindOutWhatOnePlusOneIs();

The use case is highly simplified, but it showcases the main intention: Calculator can easily switch between different external resources when needed.

---

To summarize

If you reasonably expect your logic to be universally true and never need replacing, then a static method suffices.

If you reasonably expect your dependency to be swapped at some point, or you wish to minimize code changes should a dependency unexpectedly need to be swapped; then it's better to inject the dependency using an interface so you can easily swap it out without having to change much of the code.

Answer 2

It is quite common that classes depend on other classes for some functionality. But that is usually defined by interfaces, not by static methods.

In your case, you could create an interface e.g. IWorldConverter with a (non-static, of course) method Vector3 ToWorld(Vector2Int vector, GridPlane plane).

When a PathLine gets constructed, that converter is injected, ideally via the constructor:

public PathLine(IWorldConverter converter)
{
    // store the converter in a field
}

and later on, you'd call the method of the converter instead of the static method.

That way, there is no direct coupling to the class implementing the method, and the concrete instance can be replaced for some purposes (e.g. testing).

Answer 3

The dependency on the static method creates tight coupling between PathLine and Grids.ToWorld, but we cannot really say from the question posed whether this tight coupling will lead to any trouble in your application.  If the callers do not and should not know about the Grids class or alternative conversion methods then perhaps leave it as is.

What I do find awkward with PathLine is the grouping of two mutable vector2int&vector pairs in one object — here, I'd prefer supplying two parameters in the constructor making the object immutable, and requiring callers to create a new object if anything is to change.

Let's look at the consequences of mutability in terms of the potential refactor, say, to inject an (alternative) conversion.  As is, the object goes through several potential phases:

• constructed but not initialized — since the fields are still blank,
• one field (pair) initialized
• both field (pairs) initialized
• object reused for other purposes, by mutation — related uses or totally unrelated uses

In an (injection) refactoring we have to concern ourselves in particular with the last phase, as an unrelated use maybe needs a different conversion injected, which means now a different object instance instead of reusing the old one, and potentially some additional restructuring is required since callers previously expected to be able to share a single object instance more broadly.

By contrast, working with immutable objects eliminates the last phase, and thus dealing with it during refactoring.

(There's also the ordering issue in that the object's properties can be read before being initialized.)