Является ли Reflection в базовом классе плохой дизайнерской идеей? - PullRequest
2 голосов
/ 30 июня 2009

В общем, размышления в базовом классе могут быть направлены на некоторые хорошие и полезные цели, но у меня есть случай, когда я нахожусь между молотом и наковальней ... Используйте Reflection или выставляйте открытые классы Factory, когда на самом деле должен быть закрытым семантически говоря (то есть не каждый может их использовать). Я полагаю, здесь приведен код:

public abstract class SingletonForm<TThis> : Form 
    where TThis : SingletonForm<TThis>
{
    private static TThis m_singleton;
    private static object m_lock = new object();
    private static ISingletonFormFactory<TThis> m_factory;

    protected SingletonForm() { }

    public static TThis Singleton
    {
        get
        {
            lock (m_lock)
            {
                if (m_factory == null)
                {
                    foreach (Type t in typeof(TThis).GetNestedTypes(BindingFlags.NonPublic))
                    {
                        foreach (Type i in t.GetInterfaces())
                        {
                            if (i == typeof(ISingletonFormFactory<TThis>))
                                m_factory = (ISingletonFormFactory<TThis>)Activator.CreateInstance(t);
                        }
                    }

                    if (m_factory == null)
                        throw new InvalidOperationException(string.Format(
                            CultureInfo.InvariantCulture,
                            "{0} does not implement a nested ISingletonFormFactory<{0}>.",
                            typeof(TThis).ToString()));
                }

                if (m_singleton == null || m_singleton.IsDisposed)
                {
                    m_singleton = m_factory.GetNew();
                }

                return m_singleton;
            }
        }
    }
}

Теперь этот код работает для меня, но это ужасный клудж и / или действительно плохая идея? Другая опция передает тип Factory в качестве параметра типа, но затем из-за ограничений видимости класс Factory должен быть общедоступным, что означает, что любой может вызывать его для создания экземпляров, когда они не должны быть.

Ответы [ 4 ]

3 голосов
/ 30 июня 2009

При работе с дженериками вам часто приходится использовать рефлексию. В связи с этим я думаю, что ты в порядке.

Тем не менее, я вижу здесь две формы запаха кода. Однако они могут быть связаны с обработкой кода, поэтому я просто прокомментирую их:

Во-первых, ваше статическое свойство относится к универсальному элементу. Я на 99,999% уверен, что это даже не скомпилируется. Если это так, то это дурной тон.

Во-вторых, вы, кажется, возвращаете новый экземпляр для каждого вызова Bar. Это также считается плохой формой для добытчика. Вместо этого я хотел бы иметь метод с именем CreateBar () или что-то подобное.

1 голос
/ 30 июня 2009

Эту ситуацию также лучше обрабатывать с помощью пользовательских атрибутов, в которых вы можете явно определить тип для использования.

[AttributeUsage( AttributeTargets.Class, AllowMultiple = false )]
public sealed class SingletonFactoryAttribute : Attribute
{
    public Type FactoryType{get;set;}   
    public SingletonFormAttribute( Type factoryType )
    { 
        FactoryType = factoryType; 
    }
}

Ваша синглтон-собственность теперь становится

public static TThis Singleton
{
    get
    {
        lock (m_lock)
        {
            if (m_factory == null)
            {
                var attr = Attribute.GetCustomAttribute( 
                               typeof( TThis ), 
                               typeof( SingletonFactoryAttribute ) ) 
                               as SingletonFactoryAttribute;

                if (attr == null)
                    throw new InvalidOperationException(string.Format(
                        CultureInfo.InvariantCulture,
                        "{0} does not have a SingletonFactoryAttribute.",
                        typeof(TThis).ToString()));

                m_factory = Activator.CreateInstance( attr.FactoryType );
            }

            if (m_singleton == null || m_singleton.IsDisposed)
            {
                m_singleton = m_factory.GetNew();
            }

            return m_singleton;
        }
    }
} 
0 голосов
/ 30 июня 2009

Только к вашему сведению, следующее может быть уменьшено с

foreach (Type i in t.GetInterfaces())
{
    if (i == typeof(ISingletonFormFactory<TThis>))
        m_factory = (ISingletonFormFactory<TThis>)Activator.CreateInstance(t);
}

до

if( typeof( ISingletonFormFactory<TThis> ).IsAssignableFrom( t ) )
    m_factory = Activator.CreateInstance( t ) as ISingletonFormFactory<TThis>;
0 голосов
/ 30 июня 2009

Если это возможно, я бы старался избегать использования отражения, если вы можете сойти с рук.

Чтобы сделать это, вы можете попытаться использовать шаблон Abstract Factory , чтобы обойти проблему прямого раскрытия типа общедоступной фабрики, в зависимости от вашей ситуации.

В примере из Википедии (на Java) это означает, что вы создаете интерфейс фабрики, который реализует ваш класс фабрики, затем в вашем коде создается нечто, что вам нужно, и вы возвращаете его как фабричный интерфейс.

Еще один способ сделать это - создать абстрактный класс вместо интерфейса для вашей абстрактной фабрики, а затем создать статический метод внутри этого абстрактного класса для возврата необходимой вам фабрики типов.

...