2016-07-05 2 views
0

Эта программа предназначена для проверки данных строки Excel. Если инструкция if является ложной, мне нужно добавить сообщение в строку и так далее для N чисел операторов if. Это не работает, потому что, как вы можете видеть, если первый оператор if не работает, я не буду переходить к другим операциям if. Также позже мне нужно будет обрабатывать более 100 столбцов, поэтому я ищу способ сделать это.Упростить процесс длинной вложенной операции if C#

Есть ли другой способ, я могу переписать это, чтобы он был более читабельным и менее повторяющимся? Я знаю, что могу просто сделать гигант, если (.... & & ....) для проверки всех ячеек и отдельных операторов if для добавления сообщений, но мне было интересно, есть ли другой способ сделать это. Я хотел бы сохранить порядок ошибок, но это не так важно. Результатом будет что-то вроде "facilityID is invalid, daysOfTheWeek is invalid". Я также возвращаю пользовательский тип данных Pair.

string facilityID, facilityDockDoorID, increment, hoursOfOperationId, updatedById, startTime, endTime, dockDoorServiceType, daysOfTheWeek; 
      facilityID = row[0]; 
      facilityDockDoorID = row[1]; 
      increment = row[2]; 
      hoursOfOperationId = row[3]; 
      updatedById = row[4]; 
      startTime = row[5]; 
      endTime = row[6]; 
      dockDoorServiceType = row[7]; 
      daysOfTheWeek = row[8]; 

      string errorMessage = " is invalid"; 
      if (IsInt(facilityID)) 
      { 
       if (IsInt(facilityDockDoorID)) 
       { 
        if (IsInt(increment)) 
        { 
         if (IsInt(hoursOfOperationId)) 
         { 
          if (IsInt(updatedById)) 
          { 
           if (IsTime(startTime)) 
           { 
            if (IsTime(endTime)) 
            { 
             if (IsValidDockDoorServiceType(dockDoorServiceType)) 
             { 
              if (IsValidDayOfTheWeek(daysOfTheWeek)) 
              { 
               isDataValid.First = true; 
              } 
              else 
              { 
               isDataValid.Second += "daysOfTheWeek" + errorMessage + ",";             
              } 
             } 
             else 
             { 
              isDataValid.Second += "dockDoorServiceType" + errorMessage + ","; 

             } 
            } 
            else 
            { 
             isDataValid.Second += "endTime" + errorMessage + ","; 
            } 
           } 
           else 
           { 
            isDataValid.Second += "startTime" + errorMessage + ","; 
           } 
          } 
          else 
          { 
           isDataValid.Second += "updatedById" + errorMessage + ","; 
          } 
         } 
         else 
         { 
          isDataValid.Second += "hoursOfOperationId" + errorMessage + ","; 
         } 
        } 
        else 
        { 
         isDataValid.Second += "increment" + errorMessage + ","; 
        } 
       } 
       else 
       { 
        isDataValid.Second += "facilityDockDoorID" + errorMessage + ","; 
       } 
      } 
      else 
      { 
       isDataValid.Second = "facilityID" + errorMessage + ","; 
      } 
      return isDataValid; 
     } 
+3

не гнездятся в 'if' заявления, делать их последовательно. –

+0

Возможный дубликат http://stackoverflow.com/questions/17804005/how-to-prevent-the-arrowhead-anti-pattern – nicholas

+0

@ nicholas Я посмотрел на этот вопрос. Однако я не собираюсь возвращаться из любых утверждений if. Только намереваясь вернуться после прохождения всех проверок. Самая важная часть - получение сообщения – ygongdev

ответ

5

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

if (!IsInt(facilityID)) 
{ 
    isDataValid.Second = "facilityID" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsInt(facilityDockDoorID)) 
{ 
    isDataValid.Second += "facilityDockDoorID" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsInt(increment)) 
{ 
    isDataValid.Second += "increment" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsInt(hoursOfOperationId)) 
{ 
    isDataValid.Second += "hoursOfOperationId" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsInt(updatedById)) 
{ 
    isDataValid.Second += "updatedById" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsTime(startTime)) 
{ 
    isDataValid.Second += "startTime" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsTime(endTime)) 
{ 
    isDataValid.Second += "endTime" + errorMessage + ","; 
    return isDataValid; 
} 
if (!IsValidDockDoorServiceType(dockDoorServiceType)) 
{ 
    isDataValid.Second += "dockDoorServiceType" + errorMessage + ","; 
    return isDataValid; 
} 
if (IsValidDayOfTheWeek(daysOfTheWeek)) 
{ 
    isDataValid.First = true; 
} 
else 
{ 
    isDataValid.Second += "daysOfTheWeek" + errorMessage + ","; 
} 
return isDataValid; 

Однако на основании того, что вы конкатенации к Second это более вероятно, что вы на самом деле хотите что-то вроде

if (!IsInt(facilityID)) 
    isDataValid.Second = "facilityID" + errorMessage + ","; 
if (!IsInt(facilityDockDoorID)) 
    isDataValid.Second += "facilityDockDoorID" + errorMessage + ","; 
if (!IsInt(increment)) 
    isDataValid.Second += "increment" + errorMessage + ","; 
if (!IsInt(hoursOfOperationId)) 
    isDataValid.Second += "hoursOfOperationId" + errorMessage + ","; 
if (!IsInt(updatedById)) 
    isDataValid.Second += "updatedById" + errorMessage + ","; 
if (!IsTime(startTime)) 
    isDataValid.Second += "startTime" + errorMessage + ","; 
if (!IsTime(endTime)) 
    isDataValid.Second += "endTime" + errorMessage + ","; 
if (!IsValidDockDoorServiceType(dockDoorServiceType)) 
    isDataValid.Second += "dockDoorServiceType" + errorMessage + ","; 
if (!IsValidDayOfTheWeek(daysOfTheWeek)) 
    isDataValid.Second += "daysOfTheWeek" + errorMessage + ","; 
isDataValid.First = isDataValid.Second.Length == 0; 
return isDataValid; 

Обратите внимание, что я сравниваю длину Second, чтобы определить, в случае возникновения каких-либо ошибок ,

+0

Я думал об этом как о своем последнем прибежище. Мне было интересно, могу ли я сделать что-то похожее на switch(), но для последовательных операторов if. Поскольку позже этот метод потребует более 100 операторов if для более 100 столбцов – ygongdev

+1

Если вы делаете много таких проверок, вы можете захотеть создать какую-то функцию валидатора «схемы» общего назначения, которая проходит через столбцы и сравнивается с ожидаемым типом данных, используя цикл вместо жестко закодированной функции. Вы можете поместить ожидаемые типы в список, разделенный запятыми, например «int, int, date, date, string», разделить их и переключиться внутри цикла. –

+0

@BradleyUffner, Да, я собираюсь сделать много таких проверок, поэтому я ищу лучшее решение. Я попробую ваше предложение. Тем временем я собираюсь принять это как временное решение, потому что он делает то, что я хочу, чтобы мой текущий код делал. – ygongdev

4

Вы можете инвертировать операторы if. Например:

if(!IsInt(facilityID)) 
{ 
    isDataValid.Second = "facilityID" + errorMessage + ","; 
} 
if(!IsInt(facilityDockDoorID)) 
{ 
    isDataValid.Second += "facilityDockDoorID" + errorMessage + ","; 
} 
+1

OP также должен будет добавить выражения 'return', чтобы получить такое же поведение. – juharr

+0

Разве это не значит, что мне нужно иметь 8 индивидуальных утверждений if для добавления. и тогда мне также понадобится гигант, если (... && ... && ...), чтобы проверить, проверены ли все данные? Я возвращаю пользовательскую пару struct btw. Не только логическое – ygongdev

+0

@ygongdev nope. Вы можете просто проверить, есть ли в вашем сообщении об ошибке какой-либо текст или установить другой флаг ошибки, который вы проверяете позже. –

2

Вы можете создать набор функций валидатора, например:

List<Func<string[], StringBuilder, bool>> validators = new List<Func<string[], StringBuilder, bool>>(); 

validators.Add((row, logger) => 
{ 
    string facilityID = row[0]; 

    if(IsInt(facilityID)) 
    { 
     logger.AppendLine("facilityID is invalid"); 

     return false; 
    } 

    return true; 
}); 

validators.Add((row, logger) => 
{ 
    string increment = row[2]; 

    if (IsInt(increment)) 
    { 
     logger.AppendLine("increment is invalid"); 

     return false; 
    } 

    return true; 
}); 

. . . 

Вы можете просто перебрать все так:

StringBuilder log = new StringBuilder(); 

if(validators.Any(v => v(rows, log) == false)) 
{ 
    return false; 
} 
else 
{ 
    return true; 
} 

isDataValid.Second = log.ToString(); 

Таким образом, вы можете нарушить логику валидатора до тех пор, в меньшие куски. Это позволяет не вставлять операторы if. Каждый валидатор отвечает за сбор данных, которые он хочет проверить, возвращая действительный/недействительный bool и регистрируя любые сообщения в общем StringBuilder. По мере роста вашего списка валидаторов вы можете просто добавить новую функцию в коллекцию.

+0

Мне нравится эта реализация, потому что вы можете сделать это еще на один шаг. Для суперчистого кода, в идеале, вам нужно определить вашу схему таким образом, чтобы ее можно было изменить или изменить с минимальным обновлением до этого кода. – m1m1k

0

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

public class SchemaItem 
    { 
     public int Index; 
     public string ColumnName; 
     public int ParsedValue; 

     public SchemaItem(int index, string columnName) 
     { 
      Index = index; 
      ColumnName = columnName; 
     } 
    } 
    public static Dictionary<int, SchemaItem> Schema = new Dictionary<int, SchemaItem> 
    { 
     {0, new SchemaItem(0, "facilityID")}, 
     {1, new SchemaItem(1, "facilityDockDoorID")}, 
     {2, new SchemaItem(2, "increment")}, 
     {3, new SchemaItem(3, "hoursOfOperationId")}, 
     {4, new SchemaItem(4, "updatedById")}, 
     {5, new SchemaItem(5, "startTime")}, 
     {6, new SchemaItem(6, "endTime")}, 
     {7, new SchemaItem(7, "dockDoorServiceType")}, 
     {8, new SchemaItem(8, "daysOfTheWeek")}, 
    }; 
    private static bool ParseIntAndOrWriteFailMessage(string[] row, StringBuilder logger, int index) 
    { 
     string cellContents = row[index]; 
     string columnName = Schema[index].ColumnName; 
     int intContents = -1; 
     if (int.TryParse(cellContents, out intContents)) 
     { 
      Schema[index].ParsedValue = intContents; 
      return true; 
     } 
     logger.AppendFormat("{0} is invalid,", columnName); 
     return false; 
    } 

Тогда, наконец, вы могли бы сделать:

 StringBuilder logger = new StringBuilder(); 
     foreach (var column in Schema.Select(pair => pair)) 
     { 
      ParseIntAndOrWriteFailMessage(row, logger, column.Key); 
     } 
     // Parsed values are now stored in the Schema Dictionary (which probably could use a better name) 
Смежные вопросы