54

I had an interesting discussion today with another developer about how to approach a class with a method that accepts a string and outputs string.

Imagine something like the following which is completely made up for the purpose of example

public string GetStringPart(string input)
{ 
   //Some input validation which is removed for clarity

   if(input.Length > 5)
        return input.Substring(0,1);

   if(input.Substring(0,1) == "B")
        return input.Substring(0,3);

   return string.empty;
}

A function which has some logic based on it's string input is added to a project using DI and has a DI Container in place. Would you add this new class with an interface and inject it where needed, or would you make it a static class? What are the pros and cons of each? Why would you (or not) want to make this something used with constructor injection rather than just accessed when required anywhere.

Lotok
  • 1,809

2 Answers2

53

There is no reason why this needs to be injected. This is just a function, it has no dependencies, so just call it. It can even be static if you want as it looks to be pure. One can write unit tests against this with no difficulty. If it is used in other classes, unit tests can still be written.

There is no need to abstract away functions with no dependencies, it's overkill.

If this becomes more complex then maybe passing an interface into a constructor or method is warranted. But, I wouldn't go down that road that unless I had complex GetStringPart logic based on location, etc.

Robert Harvey
  • 200,592
Jon Raynor
  • 11,773
15

Here's why:

class DOSClient {
    OrderParser orderParser;
    string orderCode;

DOSClient(OrderParser orderParser, string ordercode) { 
    this.orderParser = orderParser; 
    this.ordercode = ordercode;
}
void DisplayOrderCode() {
    Console.Write( "Prefix: " + orderParser.GetStringPart(ordercode) ); 
    ...
}



}


class GUIClient {
    OrderParser orderParser;
    string orderCode;
    GUI gui;


GUIClient(OrderParser orderParser, string ordercode, GUI gui) { 
    this.orderParser = orderParser; 
    this.ordercode = ordercode;
    this.gui = gui;
}

void DisplayOrderCode() {
    gui.Prefix( orderParser.GetStringPart(ordercode) ); 
    ...
}



}

 

class OrderParserUS : IOrderParser {

public string GetStringPart(string input) { 
    //Some input validation which is removed for clarity

    if(input.Length > 5)
        return input.Substring(0,1);

    if(input.Substring(0,1) == "B")
        return input.Substring(0,3);

    return string.empty;
}



}


class OrderParserEU : IOrderParser {


public string GetStringPart(string input) { 
    //Some input validation which is removed for clarity

    if(input.Length > 6)
        return input.Substring(0,1);

    if(input.Substring(0,1) == "#")
        return input.Substring(0,3);

    return string.empty;
}



}

If you had gone with a static method, there would be no way to change the behavior of GetStringPart without either destroying the old behavior or polluting it with conditional logic. It's true that statics are evil globals in disguise but the fact that they disable polymorphism is my chief complaint about them. Static methods aren't first class in OOP languages. By giving the method an object to live in, even one with no state, we make the method portable. Its behavior can be passed around like the value of a variable.

Here I've imagined a system that needs to behave slightly differently when deployed in Europe than when deployed in the US. Rather than force either system to contain code only needed by the other, we can instead change the behavior by controlling which order-parsing-object is injected in the clients. This allows us to contain the spread of the region detail. It also makes it easy to add OrderParserCanada without having to touch existing parsers.

If that means nothing to you, then there really isn't a good argument for this.

BTW, GetStringPart is a terrible name.

candied_orange
  • 119,268