2012-01-06 2 views
0

У меня есть следующий код не менее 10 раз в моем коде. Мне кажется вонючим.Могут ли использовать дженерики?

 public void DisplayTransitInfo(TransitInfo transitInfo) 
    { 
     if (InvokeRequired) 
      EndInvoke(BeginInvoke(new MethodInvoker(() => DisplayTransitInfo(transitInfo)))); 
     else 
     { 
      var control = (from string key in _visiblePanes.Keys 
          where key == "transitInfo" 
          select _visiblePanes[key].Control).ToList(); 

      TransitInfoControl cntl = (TransitInfoControl)control[0]; 
      //TODO: Transit Info 
     } 
    } 

    public void ModifyParties(UltraTreeNode node) 
    { 
     if (InvokeRequired) 
      EndInvoke(BeginInvoke(new MethodInvoker(() => ModifyParties(node)))); 
     else 
     { 
      var control = (from string key in _visiblePanes.Keys 
          where key == "parties" 
          select _visiblePanes[key].Control).ToList(); 

      PartiesControl cntl = (PartiesControl)control[0]; 
      cntl.ModifyParties(node); 
     } 
    } 

Я чувствую, что в этой ситуации можно использовать дженерики. Я также рассмотрел вопрос о переносе:

   var control = (from string key in _visiblePanes.Keys 
          where key == "parties" 
          select _visiblePanes[key].Control).ToList(); 

к своей собственной функции, которая вернет экземпляр элемента управления в словаре.

Этот код вонючий или мне просто нужно, чтобы мой нос работал?

Спасибо, как всегда!

ответ

3

Если вы хотите упростить

 var control = (from string key in _visiblePanes.Keys 
         where key == "transitInfo" 
         select _visiblePanes[key].Control).ToList(); 

     TransitInfoControl cntl = (TransitInfoControl)control[0]; 

и

 var control = (from string key in _visiblePanes.Keys 
         where key == "parties" 
         select _visiblePanes[key].Control).ToList(); 

     PartiesControl cntl = (PartiesControl)control[0]; 

И если _visiblePanes это словарь, который подразумевает, что не может иметь дубликаты ключей, то вам не нужны дженерики, просто использовать вместо этого:

var cntl = (TransitInfoControl)_visiblePanes["transitInfo"].Control; 

и

var cntl = (PartiesControl)_visiblePanes["parties"].Control; 

EDIT

(добавлены слепки выше для эквивалентности с исходным кодом)

Это предложение, по сравнению с Джимом Mischel, является не только проще набирать и читать, она также ближе к исходный код, потому что он генерирует исключение, если ключ отсутствует в словаре. Если исходный код использовал Linq в попытке покрыть эту возможность, он терпит неудачу при PartiesControl cntl = (PartiesControl)control[0];. Предполагая, что отсутствие ключа в словаре не является исключительным, тогда, конечно, решение TryGetValue от Jim лучше.

Чтобы вернуться к первоначальному вопросу о дженериков, там является возможное преимущество в извлечении логики Get-и отливают в общий метод:

bool TryGetCast<T>(IDictionary<string, BaseControl> dict, string key, out T value) where T : BaseControl 
{ 
    BaseControl tryGet; 
    if (dict.TryGetValue(key, out tryGet) 
    { 
     value = (T)tryGet; 
     return true; 
    } 
    value = default(T); 
    return false; 
} 

можно назвать так:

PartiesControl cntl; 
if (TryGetCast(_visiblePanes, "parties", out cntl)) 
{ 
    //do whatever; 
} 

Однако преимущество незначительное.Решение Джима плюс бросок не намного более многословен:

BaseControl c; 
if (_visiblePanes.TryGetValue("parties", out c)) 
{ 
    var cntl = (PartiesControl)c; 
    // do whatever 
} 
3

Если у вас тот же код 10 раз в вашей заявке, это определенно время для рефакторинга!

Почему вы пишете

(from string key in _visiblePanes.Keys 
         where key == "parties" 
         select _visiblePanes[key].Control) 

, когда вы можете просто использовать _visiblePanes["parties"] (или использовать TryGetValue, если вы не уверены, существует ли ключ)?

И почему есть control список? control не похоже на название коллекции.

+0

Да, если ничто иное не инкапсулирует этот код в его собственный объект. –

2

Я может быть что-то не хватает, но мне кажется, вы можете избавиться от выражения LINQ всего там. То есть, этот код:

var control = (from string key in _visiblePanes.Keys 
       where key == "parties" 
       select _visiblePanes[key].Control).ToList(); 
PartiesControl cntl = (PartiesControl)control[0]; 

Если я читаю это право, вы перечисляя шпоночную коллекцию, чтобы найти все элементы, которые имеют ключ «партию». Но так как вы пишете _visiblePanes[key].Control, кажется, что есть только один из них. Я собираюсь предположить, что _visiblePanes - это Dictionary<string, BaseControl>, где BaseControl - это класс PartiesControl и другие типы управления, наследуемые от.

BaseControl cntl; 
if (_visiblePanes.TryGetValue("parties", out cntl)) 
{ 
    PartiesControl pcntl = cntl as PartiesControl; 
    // do whatever 
} 
+0

Вы не можете передать 'PartyControl' в' IDictionary .TryGetValue'; тип переменной должен быть «BaseControl». Поэтому вам необходимо добавить бросок для доступа к любым членам подкласса. – phoog

+0

@phoog: Хороший улов. Исправленный. –

Смежные вопросы