Внутренняя глобальная собственность .. плохой запах? - PullRequest
Новая
       9

Внутренняя глобальная собственность .. плохой запах?

1 голос
/ 24 ноября 2008

У меня возникла небольшая проблема с кодом, над которым я работал:

Мой базовый код выглядит так:

Основная COM-оболочка: 

public class MapinfoWrapper 
{
    public MapinfoWrapper()
    {
        Publics.InternalMapinfo = new MapinfoWrapper();
    }

    public void Do(string cmd) 
    {
        //Call COM do command
    }

    public string Eval(string cmd)
    {
        //Return value from COM eval command
    }
}

Открытый статический класс для хранения внутренней ссылки на оболочку : 

internal static class Publics
{
    private static MapinfoWrapper _internalwrapper;
    internal static MapinfoWrapper InternalMapinfo 
    { 
        get
        {
            return _internalwrapper;     
        }
        set
        {
            _internalwrapper = value;
        }
    }
}

Код, который использует экземпляр внутренней оболочки: 

    public class TableInfo
    {
        public string Name {
            get { return Publics.InternalMapinfo.Eval("String comman to get the name"); }
            set { Publics.InternalMapinfo.Do("String command to set the name"); }
        }
    }

Это кому-нибудь плохо пахнет? Должен ли я использовать внутреннее свойство для хранения ссылки на основной объект-обертку или я должен использовать другой дизайн здесь?

Примечание. Объект MapinfoWrapper будет использоваться внешним миром, поэтому я не хочу, чтобы этот объект был единичным.

Ответы [ 2 ]

2 голосов
/ 24 ноября 2008

Вы снижаете тестируемость вашего класса TableInfo, не внедряя MapInfoWrapper в сам класс. Используете ли вы глобальный кеш этих классов MapInfoWrapper, зависит от класса - вам нужно решить, нужно ли это или нет, но это улучшит ваш дизайн, чтобы передать оболочку в TableInfo и использовать ее там, вместо прямой ссылки на глобальную копию внутри методов TableInfo. Делайте это в сочетании с определением интерфейса (т. Е. «Рефакторинг для интерфейсов»).

Я бы также сделал ленивую реализацию в получателе (ях) Publics, чтобы убедиться, что объект доступен, если он еще не был создан, вместо того, чтобы устанавливать его в конструкторе MapInfoWrapper. 

public class TableInfo
{
     private IMapinfoWrapper wrapper;

     public TableInfo() : this(null) {}

     public TableInfo( IMapinfoWrapper wrapper )
     {
          // use from cache if not supplied, could create new here
          this.wrapper = wrapper ?? Publics.InternalMapInfo;
     }

     public string Name {
        get { return wrapper.Eval("String comman to get the name"); }
        set { wrapper.Do("String command to set the name"); }
     }
}

public interface IMapinfoWrapper
{
    void Do( string cmd );
    void Eval( string cmd );
}

public class MapinfoWrapper 
{
    public MapinfoWrapper()
    {
    }

    public void Do(string cmd) 
    {
        //Call COM do command
    }

    public string Eval(string cmd)
    {
        //Return value from COM eval command
    }
}

internal static class Publics
{
    private static MapinfoWrapper _internalwrapper;
    internal static MapinfoWrapper InternalMapinfo 
    { 
        get
        {
            if (_internalwrapper == null)
            {
                _internalwrapper = new MapinfoWrapper();
            }
            return _internalwrapper;     
        }
    }
}

Теперь, когда вы тестируете методы TableInfo, вы можете легко смоделировать MapInfoWrapper, предоставив конструктору вашу собственную реализацию. Ex (при условии, что рука имитирует): 

[TestMethod]
[ExpectedException(typeof(ApplicationException))]
public void TestTableInfoName()
{
     IMapinfoWrapper mockWrapper = new MockMapinfoWrapper();
     mockWrapper.ThrowDoException(typeof(ApplicationException));

     TableInfo info = new TableInfo( mockWrapper );
     info.Do( "invalid command" );
}
0 голосов
/ 24 ноября 2008

Я думал добавить это в свой первоначальный ответ, но это действительно другая проблема.

Возможно, вы захотите подумать, должен ли класс MapinfoWrapper быть потокобезопасным, если вы храните и используете кэшированную копию. Каждый раз, когда вы используете одну глобальную копию, вам нужно учитывать, будет ли она использоваться более чем одним потоком одновременно, и создавать ее таким образом, чтобы любые критические разделы (где бы данные ни изменялись или предполагалось, что они не изменяются) являются поточными. безопасный. Если многопоточная среда должна поддерживаться - скажем, на веб-сайте - тогда это может противоречить использованию одной глобальной копии, если стоимость создания класса не очень высока. Конечно, если ваш класс опирается на другие классы, которые также не являются поточно-ориентированными, тогда вам может понадобиться сделать ваш класс в любом случае поточно-безопасным.

...