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.

asked Mar 10, 2015 at 17:18
\$\endgroup\$

1 Answer 1

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.

answered Mar 10, 2015 at 17:50
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.