прервать постановку вопроса - sonarqube

Question

Я анализирую свой код с помощью sonarqube и сталкиваюсь с ошибкой для следующего метода

public static AllocationRuleList AsAllocationRuleList(this SIGACORD.Policy acordPolicy)
    {
        foreach (var OlifeExt in acordPolicy.OLifEExtension)
        {
            var elements = new List<XmlElement>();

            foreach (var ele in OlifeExt.Any)
            {
                if (ele.Name == "AllocationRestrictions")
                {
                    var allocationRestrictionElement = acordPolicy.OLifEExtension[0]["AllocationRestrictions"];
                    return allocationRestrictionElement.AsAllocationRuleList();
                }
            }
            break;
        }
        return null;
    }

sonarqube говорит, что мой break должен быть удален или сделан условным. но разве это не логически правильно?

Answer 1

Ваш break приведет к выходу из цикла после первого элемента.Это побеждает цель цикла.

Это то же самое, что и это:

    // a for loop does nothing if there are no items in the collection
    if(acordPolicy.OLifEExtension).Any()
    {

        // no loop - we just take the first item.
        var OlifeExt = acordPolicy.First(); 
        var elements = new List<XmlElement>();

        foreach (var ele in OlifeExt.Any)
        {
            if (ele.Name == "AllocationRestrictions")
            {
                var allocationRestrictionElement = acordPolicy.OLifEExtension[0]["AllocationRestrictions"];
                return allocationRestrictionElement.AsAllocationRuleList();
            }
        }
    }
    return null;

Если вы действительно просто хотите посмотреть на первый элемент в коллекции, тогда код, который вы написали - цикл for с break после первой итерации - будет работать.Но это сбивает с толку.Кто-то должен будет прочитать все это, чтобы понять, что цикл for завершается после первого элемента.Тогда они будут удивляться, если вы хотели это сделать.Затем они прочтут еще кое-что, пытаясь выяснить, что происходит.

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

Answer 2

Ваш код является допустимым кодом C #, но сонар не о том, что компилятор.Сонар проверяет, если ваш код не имеет сбивающих с толку конструкций, а эта сбивает с толку.Вы не хотите повторять коллекцию, вы хотите только первый элемент.Таким образом, ваш код должен выражать ваше намерение этого.Таким образом, вы должны сделать

var OlifeExt = acordPolicy.OLifEExtension.FirstOrDefault();
if(OlifeExt  != null)
// ...

Answer 3

«разрыв» означает разрыв токовой петли (внешний foreach).Если искомое значение не является первым в коллекции, то вы всегда будете возвращать нуль.

Если предположить, что это не то, что вы имели в виду, что-то вроде этого должно работать лучше:

public static AllocationRuleList AsAllocationRuleList(this SIGACORD.Policy acordPolicy)
    {
        foreach (var OlifeExt in acordPolicy.OLifEExtension)
        {
            var restrictions = OlifeExt.FirstOrDefault(f => f.Name == "AllocationRestrictions");
            if (restrictions == null) continue;

            return restrictions.AsAllocationRuleList();
        }
        return null;
    }

Answer 4

Нет смысла иметь break; в любом цикле, если он не основан на каком-либо условии, например:

foreach (var OlifeExt in acordPolicy.OLifEExtension)
{
  if(OlifeExt == something)
  { 
    break;
  }

  // else continue looping and do your thing
}