Избегать кастинга несколько раз - PullRequest
5 голосов
/ 11 октября 2010

У меня есть метод, который получает параметр базового типа и выполняет некоторую предварительную обработку в зависимости от фактического типа параметра.Вот мой код:


public void OnMessageReceived(QuickFix42.Message message)
{
    if (message is QuickFix42.ExecutionReport)
    {
        ProcessExecutionReport(message as QuickFix42.ExecutionReport);
    }
    else if (message is QuickFix42.AllocationACK)
    {
        ProcessAllocationAck(message as QuickFix42.AllocationACK);
    }
    else if (message is QuickFix42.OrderCancelReject)
    {
        ProcessOrderCancelReject(message as QuickFix42.OrderCancelReject);
    }
    // ...
}

Все отлично работает, но я получаю следующее предупреждение от Visual Studio:

Warning 760 CA1800 : Microsoft.Performance : 'message', a parameter, is cast to type 'ExecutionReport' multiple times in method 'MessageProcessor.OnMessageReceived(Message)'. Cache the result of the 'as' operator or direct cast in order to eliminate the redundant isint instruction.

Какой лучший способ избежать этих избыточных приведений?

Ответы [ 7 ]

11 голосов
/ 11 октября 2010

Не используйте оба is и as. Вам просто нужно использовать as и проверить, равен ли результат нулю:

QuickFix42.ExecutionReport execReport = message as QuickFix42.ExecutionReport
if (execReport != null)
{
  ProcessExecutionReport(execReport);
}
7 голосов
/ 11 октября 2010

Учитывая повторяющуюся структуру вашего кода, вы можете очистить его следующим образом:

public void OnMessageReceived(QuickFix42.Message message)
{
    ExecuteOnlyAs<QuickFix42.ExecutionReport>(message, ProcessExecutionReport);
    ExecuteOnlyAs<QuickFix42.AllocationACK>(message, ProcessAllocationAck);
    ExecuteOnlyAs<QuickFix42.OrderCancelReject>(message, ProcessOrderCancelReject);
}

private void ExecuteOnlyAs<T>(QuickFix42.Message message, Action<T> action)
{
    var t = message as T;
    if (t != null)
    {
        action(t);
    }
}

Это предполагает, что типы не наследуются друг от друга. Если они это сделают, вам нужно изменить ExecuteOnlyAs, чтобы вернуть bool, указывающий на успех, и выполнить операторы if, как и раньше.

5 голосов
/ 11 октября 2010

Как уже сообщали другие, один as, за которым следует null -тест, будет весьма эффективным.Однако я заметил, что это приложение QuickFix.QuickFix предоставляет специально созданный класс MessageCracker, , который, я уверен, эффективно реализован, для "взлома" общих сообщений FIX в специфических объектах сообщений со строгой типизацией, которые, по-видимому, выглядят так:именно то, что вы хотите сделать. Преимущества такого подхода будут не только улучшать производительность (возможно); код будет чище (меньше проверок и приведений, а код обработки каждого сообщения будет естественным образом перемещен в соответствующий обработчик).) и более надежный в отношении недопустимых сообщений.

РЕДАКТИРОВАТЬ: взгляд на источник MessageCracker, предложенный Джоном Цвинком, заставляет задуматься о том, что использование этого класса, вероятно, приведет к повышению производительности.

Пример: (заставить ваш класс наследовать от MessageCracker)

public void OnMessageReceived(QuickFix42.Message message, SessionID sessionID)
{
  crack (message, sessionID);
}

public override void onMessage(QuickFix42.ExecutionReport message, SessionID sessionID)
{
   ...
}

public override void onMessage(QuickFix42.OrderCancelReject message, SessionID sessionID)
{
   ...
}
2 голосов
/ 11 октября 2010

По-моему, вам не хватает чего-то в вашем дизайне.Как правило, я видел нечто похожее на это раньше, в результате чего это могло произойти:

public interface IResult
{
  // No members
}

public void DoSomething(IResult result)
{
  if (result is DoSomethingResult)
    ((DoSomethingResult)result).DoSomething();
  else if (result is DoSomethingElseResult)
    ((DoSomethingElseResult)result.DoSomethingElse();
}

Поскольку контракт не определяет какие-либо операции, он заставляет вас выполнять приведение типов, чтобы получить какие-либо выгоды от него.Я не думаю, что это правильный дизайн.Контракт (будь то интерфейс или абстрактный класс) должен определять предполагаемую операцию, и должно быть понятно, как ее использовать.В приведенном выше примере, по сути, та же проблема: Message не определяет способ его использования ... вы должны изменить Message для принудительного выполнения какой-либо операции:

public abstract class Message
{
  public abstract void Process();
}

public class ExecutionReportMessage : Message
{
  public override void Process()
  {
    // Do your specific work here.
  }
}

С этим,Вы значительно упрощаете реализацию:

public void OnMessageReceived(QuickFix42.Message message)
{
    if (message == null)
      throw new ArgumentNullException("message");

    message.Process();
}

Это намного чище, более тестируемо и не приводит к типу.

2 голосов
/ 11 октября 2010
QuickFix42.ExecutionReport executionReportMessage = message as QuickFix42.ExecutionReport;
if (executionReportMessage != null) 
{ 
  ProcessExecutionReport(executionReportMessage); 
} 
1 голос
/ 17 октября 2011
public void OnMessageReceived(QuickFix42.Message message)
{
    QuickFix42.ExecutionReport executionReport;
    QuickFix42.AllocationACK allocationAck;
    QuickFix42.OrderCancelReject orderCancelReject;

    if ((executionReport = message as QuickFix42.ExecutionReport) != null)
    {
        ProcessExecutionReport(executionReport);
    }
    else if ((allocationAck = message as QuickFix42.AllocationACK) != null)
    {
        ProcessAllocationAck(allocationAck);
    }
    else if ((orderCancelReject = message as QuickFix42.OrderCancelReject) != null)
    {
        ProcessOrderCancelReject(orderCancelReject);
    }
    // ...
}
1 голос
/ 11 октября 2010

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

public void OnMessageReceived(QuickFix42.Message message)  
{  
      QuickFix42.ExecutionReport ExecutionReportMessage = message as QuickFix42.ExecutionReport;
      QuickFix42.AllocationACK  AllocationACKMessage = message as QuickFix42.AllocationACK  ;
      QuickFix42.OrderCancelReject OrderCancelRejectMessage = message as QuickFix42.OrderCancelReject;

 if (ExecutionReportMessage !=null)  
{  
    ProcessExecutionReport(ExecutionReportMessage);  
}  
else if (AllocationACKMessage !=null)  
{  
    ProcessAllocationAck(AllocationACKMessage );  
}  
else if (OrderCancelRejectMessage !=null)  
{  
    ProcessOrderCancelReject(OrderCancelRejectMessage);  
}  
// ...  

}

...