Custom string formatters

Question

Some time ago I was experimenting with an Extendable format provider but I wasn't satisfied with the implementation. It didn't feal very SOLID so I've rewritten it to this:

The base class is the Formatter:

public abstract class Formatter : IFormatProvider, ICustomFormatter
{
    protected Formatter(CultureInfo culture = null)
    {
        Culture = culture ?? CultureInfo.InvariantCulture;
    }

    public static Formatter Default(CultureInfo culture = null) => new CompositeFormatter(culture);

    public CultureInfo Culture { get; }

    public virtual object GetFormat(Type formatType)
    {
        // null means use the default .net formatter
        return formatType == typeof(ICustomFormatter) ? this : null;
    }
    public abstract string Format(string format, object arg, IFormatProvider formatProvider);
}

It supports culture info and by default the invariant culture. As we already have to implement the Format method I didn't want to create another one like TryFormat and if the formatting didn't work I return null... but maybe I should have go the TryFormat way?

As an example here are three formatters:

public class DecimalColorFormatter : Formatter
{
    public override string Format(string format, object arg, IFormatProvider formatProvider)
    {
        if (!(arg is Color))
        {
            return null;
        }

        var color = (Color)arg;
        var argb = color.ToArgb();

        if (format.Equals("rgb-dec", StringComparison.OrdinalIgnoreCase))
        {
            return String.Format(Culture, "({0},{1},{2})", color.R, color.G, color.B);
        }

        if (format.Equals("argb-dec", StringComparison.OrdinalIgnoreCase))
        {
            return String.Format(Culture, "({0},{1},{2},{3})", color.A, color.R, color.G, color.B);
        }

        return null;
    }
}

public class HexadecimalColorFormatter : Formatter
{
    public override string Format(string format, object arg, IFormatProvider formatProvider)
    {
        if (!(arg is Color))
        {            
            return null;
        }

        var color = (Color)arg;
        var argb = color.ToArgb();

        if (format.Equals("rgb-hex", StringComparison.OrdinalIgnoreCase)) 
        {
            return String.Format(Culture, "#{0:X2}{1:X2}{2:X2}", color.R, color.G, color.B);
        }

        if (format.Equals("argb-hex", StringComparison.OrdinalIgnoreCase))
        {
            return String.Format(Culture, "#{0:X2}{1:X2}{2:X2}{3:X2}", color.A, color.R, color.G, color.B);
        }

        return null;
    }
}

public class CaseFormatter : Formatter
{
    public override string Format(string format, object arg, IFormatProvider formatProvider)
    {
        switch (format)
        {
            case "U": return arg.ToString().ToUpper();
            case "L": return arg.ToString().ToLower();
            default: return null;
        }
    }
}

I compose the formatter with this two helpers:

public class CompositeFormatter : Formatter
{
    private Formatter[] _formatters;

    public CompositeFormatter(CultureInfo culture, params Formatter[] formatters) : base(culture)
    {
        _formatters = formatters;
    }

    public CompositeFormatter(params Formatter[] formatters)
    : this((CultureInfo)null, formatters)
    { }

    public override string Format(string format, object arg, IFormatProvider formatProvider)
    {
        foreach (var formatter in _formatters)
        {
            var result = formatter.Format(format, arg, formatProvider);
            if (result != null)
            {
                return result;
            }
        }

        return null;
    }
}

public static class FormatterComposer
{
    public static Formatter Add<T>(this Formatter formatter, CultureInfo culture = null)
    where T : Formatter, new()
    {
        return new CompositeFormatter(culture, formatter, new T());
    }
}

---

Examples:

var formatter = Formatter.Default()
    .Add<DecimalColorFormatter>()
    .Add<HexadecimalColorFormatter>()
    .Add<CaseFormatter>();


string.Format(formatter, "{0:U} = {0:rgb-hex} or {0:rgb-dec}", Color.Aquamarine);
string.Format(formatter, "{0:L} = {0:argb-hex} or {0:argb-dec}", Color.Aquamarine);
string.Format(formatter, "{0} = {0:argb-hex}", (string)null);

results:

COLOR [AQUAMARINE] = #7FFFD4 or (127,255,212)
color [aquamarine] = #FF7FFFD4 or (255,127,255,212)
 =

Answer 1

Unused variables

First of all, there are argbvariables in your code which are not used, so let`s remove them.

Type keywords

Secondly you use String class specifier instead of string, while the second one is strongly suggested by Microsoft. I would have changed that for your code to be consistent with code of other .NET developers.

Unnecessary constructor

Thirdly, if you use default parameters like this in the first constructor of CompositeFormatter

public CompositeFormatter(CultureInfo culture = null, params Formatter[] formatters) : base(culture)
{
    _formatters = formatters;
}

then you have no need for second one and it can be removed.

Creation of not required objects

Fourthly, your Add<T> extension create new instance of CompositeFormatter every time and they are kept in memory which is not a good practice. And the execution travels through a lot of objects because of this.

So what we can do is rewrite Formatter[] _formatters to List<Formatter>. Then in constructor _formatters = formatters.ToList(). Later we can add next method to CompositeFormatter

public void Add(Formatter formatter)
{
    _formatters.Add(formatter);
}

Finally, we can rewrite your extension like this

public static Formatter Add<T>(this Formatter formatter, CultureInfo culture = null)
    where T : Formatter, new()
{
    var newFormatter = new T();
    var compositeFormatter = formatter as CompositeFormatter;
    if (compositeFormatter == null)
    {
        return new CompositeFormatter(culture, formatter, newFormatter);
    }
    compositeFormatter.Add(newFormatter);
    return compositeFormatter;
}

Few final notes.

Your _formatters should be marked as readonly since you are only assigning value to it in the constructor.

Make classes that are not intended for inheritance sealed not only because it improves the code style and prevents design flaws but also because it improves performance.

Best regards :)