Is there a name for this technique and is it a code smell?

user_2437916 0
Viewed 338

I've seen code like the following before, which results in a sort of "extension access method" being added to an object. The extension method will appear in intellisense as only one method, but when selected, intellisense will appear with all the methods defined in the "manager" class. It seems like a nice way of organizing a set of similar functionality and decluttering the main intellisense of the primary object.

So, I'm wondering if this technique has some sort of commonly used name, and also whether it's considered a code smell (beyond the general problem of the primary object taking on too much responsibility and getting too large).

public class StringManager
{
    public StringManager(String value)
    {
        Value = value;
    }

    private String Value { get; set; }

    public int GetTwiceLength()
    {
         return Value.Length * 2;
    }

    public decimal GetHalfLength()
    {
         return Value.Length / 2;
    }
}

public static class StringExtensions
{
    public static StringManager Operations(this String value)
    {
        return new StringManager(value);
    }
}

the above code would be used like so:

var myString = "the string";
var twiceLength= myString.Operations().GetTwiceLength();

Apologies for the silly functionality. This was borrowed from an example on SO where the technique was actually recommended, modified to protect the potentially guilty.

extension-methods c#
1 Answers

This looks like the adapter pattern.

Adapter is a structural design pattern that allows objects with incompatible interfaces to collaborate. – [https://refactoring.guru/design-patterns/adapter]

Let me explain why I think that applies here:

Extension methods in C# are just regular static methods with just a little bit of syntactic sugar. The this keyword for the first parameter of the function allows you to call it as if it were an instance method on that type. But you could just as well call it like a regular static method:

StringExtensions.Operations("your string").CallMethtod();

Looking at the Operations implementation:

public static StringManager Operations(this String value)
{
    return new StringManager(value);
}

We can see that this is a plain old static factory method which returns an instance of your Manager class (= the adapter). This is equivalent to creating the apapter instance directly (but actually favorable, since it hides "how" new instances are created. You could for example pool and reuse existing instances):

new StringManager("your string").CallMethod();

Your Manager class adapts the string type/interface to be compatible with a different interface, providing all your "management methods" instead of the original string methods. To make the "adapter" explicitly visible, it helps to use an intermediate variable (for demonstration purposes):

var originalString = "your string"; var adaptedString = new StringManager(originalString); adaptedString.CallMethod();

Is it a code smell? I don't think so, not necessarily. You are only adapting the interface of one class to a different interface (string interface to StringManager interface). One might argue that "Manager" should not be used in class names, because it does not add any helpful context and could basically mean anything. Sooner or later all your classes will be some sort of Manager or Service or ManagerService.

As for all patterns: use them when they make sense and provide benefit. Do not overuse them: Do not use design patterns just for the sake of using a pattern. Sometimes it is even better to not use a pattern, even if one would exist or to introduce smells on purpose (use code comments to explain why something was implemented in that way). Use sensible judgement.

user_1129680
Related