Лучшая практика - две функции с аналоговым значением - PullRequest
0 голосов
/ 16 сентября 2018

Я хочу знать, что является лучшей практикой для этой ситуации. Возьми эти две функции

Опция 1 - TableController.cs

void deleteRow(Row myRow) {
    // do some control on myRow
    if (!(myRow.hasThatValue()) && (myRow.Title.StartsWith("xxx"))
        return;

    Rows.Remove(myRow);
    Events.OnRemoveRow(myRow);
}


void deleteRows(List<Row> myRows) {
   foreach (var r in myRows) {
       deleteRow(r);     // That call each time OnRemoveRow()
   }
}

Опция 2 - TableController.cs

void deleteRow(Row myRow) {
    // do some control on myRow
    if (!(myRow.hasThatValue()) && (myRow.Title.StartsWith("xxx"))
        return;

    Rows.Remove(myRow);
    Events.OnRemoveRow(myRow);   
}


void deleteRows(List<Row> myRows) {
   foreach (var r in myRows) {
        // do some control on myRow
        if (!(r.hasThatValue()) && (r.Title.StartsWith("xxx"))
            continue;

        Rows.Remove(myRow);
   }

   Events.OnReloadTable();     // That call only one time OnReloadTable()
}

Можете ли вы заметить разницу? Скажем, у вас есть 1000 строк для удаления, и в случае OnRemoveRow вы удаляете строку в ListView, вместо этого в событии OnReloadTable вы очищаете и перезагружаете все строки в ListView.

С опцией 1 вы вызываете 1000 раз событие, которое выполняет большую работу в графическом интерфейсе, вместо этого с опцией 2 вы вызываете только один раз событие, которое перезагружает все строки.

Использование эталонного теста, очевидно, означает, что вариант 2 работает намного лучше.

Итак, вопрос:

Есть альтернативный или лучший способ, который повторяет два раза один и тот же код, как в варианте 2, но с хорошей производительностью этого решения?

Изменить:

@ Ulf: в примере исправлено возвращение в цикле с помощью continue.

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

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

Лично первое решение зачисляется на: Яир Хальберштадт

Наличие внутреннего метода может решить избыточность кода. Второе решение - это ответ Mureinik: переместите логику удаления строки в удаление списка строк и всегда используйте список строк, но создавайте каждый раз, когда новый список одного элемента кажется немного перегруженным.

Ответы [ 3 ]

0 голосов
/ 16 сентября 2018

Добавьте третий метод deleteRowInternal, содержащий разделяемую логику.

 public void deleteRow(Row myRow) {
    deleteRowInternal(myRow);

    Events.OnRemoveRow(myRow);
}

private void deleteRowInternal(Row myRow) {
     // do some control on myRow
    if (!(myRow.hasThatValue()) && 
     (myRow.Title.StartsWith("xxx"))
         return;
    Rows.Remove(myRow);
}


 public void deleteRows(List<Row> myRows) {
       foreach (var r in myRows) {
       deleteRowInternal(r); 
     }
     Events.OnReloadTable();
 }
0 голосов
/ 17 сентября 2018

Ваш второй вариант несовместим с первым;при условии, что 1st является правильным (вы хотите удалить все rows, которые имеют это значение и не начинаются с "xxx"):

   ...
   foreach (var r in myRows) {
        // do some control on myRow
        if (!(myRow.hasThatValue()) && (myRow.Title.StartsWith("xxx"))
            return; // <- should be "continue";

        Rows.Remove(myRow); // <- should be "r"
   }
   ...

В порядкечтобы избежать таких ошибок, я предлагаю запросить с помощью Linq

// Let's change List<T> into more generic IEnumerable<T>
void deleteRows(IEnumerable<Row> myRows) {
   // Readbility: let's put it clear what we are going to remove
   var toRemove = myRows
     .Where(row => !row.Title?.StartsWith("xxx"))
     .Where(row => row.hasThatValue());

   // If you have RemoveRange in Rows collection - remove in one go
   // Rows.RemoveRange(toRemove);

   // If you don't have RemoveRange method, let's loop
   foreach (var r in toRemove)
     Rows.Remove(r);

   Events.OnReloadTable();     // That call only one time OnReloadTable()
}

// Taken from Mureinik's answer (except the collection class wrapper)
void void deleteRow(Row myRow) {
  deleteRows(new Row[] {myRow}); 
}
0 голосов
/ 16 сентября 2018

Как вы заметили, вариант № 2 будет работать лучше, но он повторяет много кода между двумя методами и приводит к трудной поддержке кодовой базы.

Я бы выбрал третий вариант, где deleteRows содержит более эффективную логику варианта # 2, а deleteRow использует его:

void deleteRow(Row myRow) {
    deleteRows(new List<Row>(){myRow});
}


void deleteRows(List<Row> myRows) {
   foreach (var r in myRows) {
        // do some control on myRow
        if (!(myRow.hasThatValue()) && (myRow.Title.StartsWith("xxx"))
            return;

        Rows.Remove(myRow);
   }

   Events.OnReloadTable();
}
...