1
\$\begingroup\$

I have an implementation of a factory like so:

public class CommandActionFactory : IFactory<ICommandAction, XElement>
{
    private readonly IDictionary<string, IFactory<ICommandAction, XElement>> commandActionFactories;

    public IDictionary<string, IFactory<ICommandAction, XElement>> CommandActionFactories
    {
        get
        {
            return commandActionFactories;
        }
    }

    public CommandActionFactory(IDictionary<string, IFactory<ICommandAction, XElement>> factories)
    {
        if (factories == null)
        {
            throw new ArgumentNullException("factories");
        }

        this.commandActionFactories = factories;
    }

    public ICommandAction Create(XElement parameter)
    {
        if (parameter == null)
        {
            throw new ArgumentNullException("parameter");
        }

        return CommandActionFactories[parameter.Name.LocalName].Create(parameter);
    }
}

This factory picks between factories of the same type, based on the name of the root XML element passed to it. This means I can have bespoke factories for different XML structures, while still referring to only one top-level factory.

FXCop flags the constructor and public getter with the following warning:

CA1006 Do not nest generic types in member signatures.

So I tried simplifying the generic type as follows:

public interface IXmlCommandActionFactory : IFactory<ICommandAction, XElement>
{
}

public class CommandActionFactory : IXmlCommandActionFactory
{
    private readonly IDictionary<string, IXmlCommandActionFactory> commandActionFactories;

    public IDictionary<string, IXmlCommandActionFactory> CommandActionFactories
    {
        get
        {
            return commandActionFactories;
        }
    }

    public CommandActionFactory(IDictionary<string, IXmlCommandActionFactory> factories)
    {
        if (factories == null)
        {
            throw new ArgumentNullException("factories");
        }

        this.commandActionFactories = factories;
    }

    public ICommandAction Create(XElement parameter)
    {
        if (parameter == null)
        {
            throw new ArgumentNullException("parameter");
        }

        return CommandActionFactories[parameter.Name.LocalName].Create(parameter);
    }
}

However I'm wondering whether, while it makes the issue go away, the new solution is actually hampering readability by hiding the expected type behind another interface.

Which do you think is more readable? This is intended to be a public API.

\$\endgroup\$
1
\$\begingroup\$

You can ignore and suppress this warning in your case. This warning is intended to force you to have Dictionary<TypeWithReasonableName1, TypeWithReasonableName2> instead of Dictionary<Action<int>, List<Enumerable<string>>>, but your code is readable enough and shall not produce confusion.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.