Структура кода, методы должны делать только одно

Question

Я немного придерживаюсь того, что «метод должен делать только одну вещь»

У меня есть текстовый файл автомобиля, и если он содержит хотя бы один BMW, я хочу установить для isValid значение true, но пока яВ любом случае, я собираюсь просмотреть текстовый файл. Я также хотел бы заполнить две верхние модели списка (M3, M5 и т. д.) и младшую модель (335, X3 и т. д.).

Я знаю, что метод должен делать только одну вещь, но для него также очень удобно заполнять списки.Вот что у меня есть:

private bool hasBMWegments()
{
   foreach (ClassLib.CarSegment carElement in CarSegmentFactory.ContainsCar("BMW"))
   {
     isValid = true;
     if (carElement.Class.IndexOfAny(lowerModels) == 0)
     {
        lstOlderSegment.Add(carElement.ElementNumber);
     }
     if (carElementClass.IndexOfAny(upperModels) == 0)
     {
        lstNewerSegment.Add(carElement.ElementNumber);
     }
   }
   return isValid;
 }

Должен ли я просто создать метод, который снова выполняет проверку foreach?Или я должен создать другой метод внутри этого метода (я думаю, что это будет грязно, и не будет связано с именем метода)

edit: извините, работа с фреймворком 2.0

Answer 1

Я считаю этот код беспорядком по сравнению с этим:

private IEnumerable<ClassLib.CarSegment>
    GetModels(IEnumerable<ClassLib.CarSegment> segments, string modelID)
{
    return segments.Where(x => x.Class.IndexOfAny(modelID) == 0);
}

// ...

var bmwSegments = CarSegmentFactory.ContainsCar("BMW").ToArray();

bool isValid = bmwSegments.Any();
var olderModelSegments = GetModels(bmwSegments, lowerModels);
var newerModelSegments = GetModels(bmwSegments, upperModels);

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

Answer 2

Похоже, все, что вы делаете, это устанавливаете isValid в значение true при первом проходе через foreach. Таким образом, все isValid действительно означает «есть хотя бы один элемент?».

В этом случае вам не нужно повторять дважды. Вы можете использовать Any() для проверки:

bool IsValid(IEnumerable<CarSegment> elements)
{
    return elements.Any();
}

void PopulateSegments(IEnumerable<CarSegment> elements)
{
    foreach(var element in elements)
    {
        //add to lists
    }
}