2013-04-14 4 views
0

Этот код извлекает строки файла журнала и пытается найти дату в одном из разделенных полей. Какой символ является разделителем, он может меняться, а также в каком поле можно найти дату. Он также может быть записан в разных форматах дат или в миллисекундах с момента времени Unix. Он начинается со дна списка, и если он не найден, он возвращается к рекурсивно, пока не будет больше строк для чтения. Итак, самый простой и наиболее читаемый способ сделать это (на мой взгляд) - это использовать вложенные блоки try-catch, но учитывая, что это также рекурсивный метод, теоретически должен давать более низкую производительность? Это плохой код?Является ли удобочитаемость кода оправданием вложенных блоков try-catch для управления потоком?

Я могу использовать кучу блоков if-else вместе с DateTime.TryParse с дополнительными переменными для результата, но это заставило бы мои глаза кровоточить. Обеспечивает ли удобочитаемость кода обоснование вложенных блоков try-catch для управления потоком?

static DateTime? Search(List<string> lines) 
{ 
    if (lines.Count == 0) 
    { 
     return null; 
    } 

    DateTime? date = null; 
    string dateField; 

    try 
    { 
     dateField = lines.Last().Split(';')[18].Trim('\'').Trim(); 
     date = DateTime.ParseExact(dateField, "MM'/'dd'/'yy HH:mm:ss", null, DateTimeStyles.AllowWhiteSpaces); 
    } 
    catch (Exception) 
    { 
     try 
     { 
      dateField = lines.Last().Split(';')[19].Trim('\'').Trim(); 
      date = DateTime.ParseExact(dateField, "MM'/'dd'/'yy HH:mm:ss", null, DateTimeStyles.AllowWhiteSpaces); 
     } 
     catch (Exception) 
     { 
      try 
      { 
       dateField = lines.Last().Split(':')[9].Split('=')[1].Trim(); 
       date = FromUnixEpochTime(long.Parse(dateField)); 
      } 
      catch (Exception) 
      { 
       try 
       { 
        dateField = lines.Last().Split(':')[12].Split('=')[1].Trim(); 
        date = FromUnixEpochTime(long.Parse(dateField)); 
       } 
       catch (Exception) 
       { 
        try 
        { 
         dateField = lines.Last().Split(':')[19].Trim('\'').Trim(); 
         date = DateTime.ParseExact(dateField, "MM'/'dd'/'yy HH", null, DateTimeStyles.AllowWhiteSpaces); 
        } 
        catch (Exception) 
        { 
         lines.RemoveAt(lines.Count - 1); 
         date = Search(lines); 
        } 
       } 
      } 
     } 
    } 

    return date; 
} 
+0

Прежде всего, вы не должны улавливать общие типы исключений, например 'Exception' – MarcinJuraszek

+2

. Вам действительно стоит рассмотреть возможность поиска в TryParse и TryParseExact, которые облегчат большую часть ваших забот об исключениях. –

ответ

2

Я знаю, что ты управлял их, но я бы с

TryParse 

и

TryParseExact 

(хотя AFAIK внутренне они просто используют примерочных поймать .. но ваш код станет более читаемым)

Я бы заимствовал идею у TryParse и т. д. и сделал что-то по строкам .. (см. ниже)

Я думаю, что этот стиль отделяет каждую стратегию и делает ее более ремонтопригодной, поскольку существует меньше «шума» (т.исключение ловли) вокруг каждой стратегии

(я позволил дублирования в примерах стратегий, но некоторые, которые могут быть повторно вынесена а)

приветствий

Stu

static DateTime? Search(List<string> lines) 
{ 
    if (lines.Count == 0) 
    { 
     return null; 
    } 

    DateTime parsedDate; 

    // the first successful strategy will 'short circuit' the rest so they don't run 
    if(TryGetDateStrategy1(lines, out parsedDate) 
     || TryGetDateStrategy2(lines, out parsedDate) 
     || etc. etc.) 
    { 
     return parsedDate; 
    } 
    return null; 
} 

private static bool TryGetDateStrategy1(List<string> lines, out DateTime? date) 
{ 
    var dateField = lines.Last().Split(';')[19].Trim('\'').Trim(); 
    DateTime parsedDate; 
    if (DateTime.TryParseExact(dateField, "MM'/'dd'/'yy HH:mm:ss", null, DateTimeStyles.AllowWhiteSpaces,out date)) 
    { 
     date = parsedDate; 
     return true; 
    } 
    return false; 
} 
private static bool TryGetDateStrategy2(List<string> lines, out DateTime? date) 
{ 
    var dateField = lines.Last().Split(';')[18].Trim('\'').Trim(); 
    DateTime parsedDate; 
    if (DateTime.TryParseExact(dateField, "MM'/'dd'/'yy HH:mm:ss", null, DateTimeStyles.AllowWhiteSpaces, out date)) 
    { 
     date = parsedDate; 
     return true; 
    } 
    return null; 
} 
+0

Отличное предложение! Думаю, я заберу свой путь. Спасибо! – dakt

0

Исключения предназначены для «исключительных» случаев. У них плохое время исполнения, так как они обрабатываются иначе, чем обычный код. В Интернете есть много объяснений, объясняющих, почему исключения замедляют работу кода.

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

Также используйте строковые константы вместо того, чтобы снова вводить формат даты. Это создает код, который сложнее поддерживать.

В качестве подумав вы можете использовать только если заявления:

if (tryParse(...)){ 
    return Parse(...); 
} 

if (tryParse(...)){ 
    return Parse(...); 
} 

return null; 

Это было бы самым читаемым без вложенности и может быть прокомментированы в более читаемом виде

+0

Почему вы разобрали его дважды? Это может привести к появлению горлышек для бутылок, особенно в плотных петлях. Если tryparse {возвращаемое значение разбирается из метода tryparse}. – tsells

+0

@ Geek есть группа различных строк, проверенных на даты, каждый, если цикл представляет собой тест для одного из них. (в основном, развёртывание исходной структуры) –

2

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

public DateTime? GetValue() 
    { 
     DateTime? value = null; 

     // Regular way 
     value = GetValueImpl1(); 

     if (value != null) 
      return value; 

     // Fall back 1 
     value = GetValueImpl2(); 
     if (value != null) 
      return value; 

     // Fall back 2 
     value = GetValueImpl3(); 
     if (value != null) 
      return value; 
     return null; 
    } 
    private DateTime? GetValueImpl1() 
    { 
     return new DateTime(); 
    } 
    private DateTime? GetValueImpl2() 
    { 
     return new DateTime(); 
    } 
    private DateTime? GetValueImpl3() 
    { 
     return new DateTime(); 
    }