Utility method with several parameters, that have same values, that does not belong in any class. Where to put?

Question

I have a problem placing and shaping the flowing method in my program:

private void PrintWarning(params string[] messages)
{
    if (!_suppressWarnings)
    {
        if (_warningsAsErrors)
        {
            foreach (string message in messages)
            {
                Console.WriteLine($"Error: {message}");
            }
            Environment.Exit(1);

        }
        else
        {
            foreach (string message in messages)
            {
                Console.WriteLine($"Warning: {message}");
            }
        }
    }
}

This is C# but it can as well be java or another high level OOP language.

I already moved it between three different places and it feels wrong in each case. Note, that it's not a practical question as such: for a simple program like mine, it does not matter much where I put it, it will work in any case. But I'd like to find out, what changes I need to make in order for it to appear logical from OOP and class design perspectives.

The program is a command line utility, with many command line flags, and _suppressWarnings and _warningsAsErrors represent two of these. They normally reside in the Command class:

class Command
{
    public bool SuppressWarnings { get; set; }
    public bool WarningsAsErrors { get; set; }
    //... other command line options follow

The command line utility is used in a CI/CD automation pipeline, and in this mode WarningsAsErrors option will be set, so that the process bails out as soon as possible if there is even a single warning. This utility will be used locally while building a pipeline, so without WarningsAsErrors set it can display many warnings. Once the pipeline is build it's not likely to change, so bailing out on the first warning and not displaying the rest makes sense and is the safest.

Command line options in form if a Command object are passed to an instance of the Processor class and saved as a private field in it. The Processor class works off a TOML file, so the first thing it does when it gets control is read and parse that TOML file with help of third party library. TOML as such does not define "schema", but only certain structure of the TOML file and only certain types make sense for the command line utility, so after reading the TOML file, the Processor class constructs a TomlTypeChecker class and passes the instance of the TOML object to it for validating this ephemeral schema:

TomlTypeChecker typeChecker = new TomlTypeChecker(
  _command.SuppressWarnings, 
  _command.WarningsAsErrors, 
  //... some other parameters);
typeChecker.CheckTypes(toml);

The PrintWarning method in the incarnation shown above, is a method of the TomlTypeChecker class.

Unfortunately, the Process class also needs this same logic in PrintWarning, because sometimes it can detect likely errors in the TOML file based on data it sees, which are not related to schema. Clearly, TomlTypeChecker class is not a good home for this method, as it could be use elsewhere.

I can, of course add warningsAsErrors and suppressWarnings parameters to the method, and move it to a static method in a utility class. This is somewhat unsatisfying because this method is being called a few dozens of times in the program, and specifying the same warningsAsErrors and suppressWarnings each time feels wrong. But putting it in a base class does not feel right either, because let's be honest, Processor and TomlTypeChecker logically completely independent, they don't have the same parent.

What is the best way to solve this? I'm seriously considering making Command a static property. Is this the best solution?

Answer 1

Logging Object

Instead of instructing the process on what messages to generate, pass it an object to report too.

This is usually called a logging object. This object can be configured to capture and direct different logging to different logs.

Eg: errors can be directed to a highlights log, and the rest to a detailed log. Or even ignore certain messages, or suppress them if many similar messages are being generated.

Answer 2

Nothing is de jure correct in Object-Oriented Programming. A small cost-benefit analysis will ensure that you can make the most informed decision, as well as that you do understand the whys behind each approach:

1. Static Method

   • Benefits:
     • No code duplication
   • Costs:
     • Low amenity to refactoring. Every class that uses the extension method must be changed (cf. broken) to use a potentially different method, if something changes in the future.
     • May need to support multiple requirements, thus polluting the method with many, sometimes optional arguments, since the method has to serve multiple consumers.
     • High coupling risk potential (a static method also forms an "entry point" to your public API, thus creating non-breakage expectations and responsibilities), only for public methods, of course.

2. Within the using class (i.e. Command)

   • Benefits:
     • Clean, privately owned method, no fear of breaking up existing code while refactoring.
   • Costs:
     • May need to duplicate code if a future service needs the same or a similar facility/functionality.
     • May increase responsibilities of the containing class (see Single Responsibility Principle). This may cause the containing class to begin "exposing" properties to offer the additional functionality.

3. Behind an interface, as an injectable dependency.

   • Benefits:
     • Decreases coupling, as the consumer classes don't need to expose additional properties or anything else, i.e. there is no responsibility-load increase involved.
     • Provides extensibility to consumers (someone may have a different idea about how they would like your method to behave, though you can always provide a "default" implementation).
   • Costs:
     • Time for consideration and implementation, potential need to alter an already-existing functional design (i.e. you may need to refactor to introduce it, thus breaking/changing consumers).

I am certain that I have missed various, maybe even significant, considerations, but you get the idea. Note down what and, most importantly, why!

And, by the way Environment.Exit(1); is not a good idea for the method. Simply provide a parameter that says what to do when a warning is encountered:

void PrintWarning(Action warningEncountered, params string[] warnings)
{
    //...
    warningEncountered.Invoke();
    //...
}

Otherwise, you are, again, increasing the responsibilities of the class that contains the method and, not only that, but also in a rather offensive way! At the very least, have a bool FailFastOnWarning { get; } sort of thing.