Рекурсивный код трудно понять - нужно упростить

Question

У меня есть сущность, и я могу иметь InvoiceLine поверх этого, тогда вы можете кредитовать эту линию счетов бесконечно бесконечно. но только основная InvoiceLine имеет ссылку на исходный объект.

Я использую рекурсии, чтобы получить исходный объект, но код не то, что читаемый

private static PermanentPlacement PopulatePermanentPlacement(InvoiceLine invoiceLine) 
     { 
      PermanentPlacement permanentPlacement; 
      var creditReissue = invoiceLine.CreditReissue; 
      do 
      { 
       permanentPlacement = creditReissue.InvoiceLine.PermanentPlacement; 
       if (permanentPlacement == null) 
       { 
        creditReissue = creditReissue.InvoiceLine.CreditReissue; 
       } 
      } while(permanentPlacement == null); 

      return permanentPlacement; 
     } 

Есть ли способ, что я могу сделать это более понятным и упростить?

Answer 1

Я думаю, что ответ Рене Фогта показывает лучший способ упростить этот код. Но есть и другие. Например, рассмотрим перемещение петли на вспомогательную функцию:

static IEnumerable<Reissue> CreditReissues(Reissue original) 
{ 
    var current = original; 
    while(true) 
    { 
    yield return current; 
    current = current.InvoiceLine.CreditReissue; 
    } 
} 

И теперь вы никогда не должны написать цикл снова для того, чтобы использовать бесконечную последовательность кредитных переизданий:

private static PermanentPlacement PopulatePermanentPlacement(
    InvoiceLine invoiceLine) 
{ 
    return CreditReissues(invoiceLine.CreditReissue) 
    .Select(cr => cr.InvoiceLine.PermanentPlacement) 
    .First(pp => pp != null); 
} 

То есть: возьмите бесконечную последовательность кредитных повторов, превратите ее в бесконечную последовательность постоянных мест размещения и верните первый ненулевой в последовательности.

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

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

private static PermanentPlacement PopulatePermanentPlacement(
    InvoiceLine invoiceLine) 
{ 
    return invoiceLine.PermanentPlacement ?? 
     PopulatePermanentPlacement(
      invoiceLine.CreditReissue.InvoiceLine); 
} 

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

Answer 2

Я бы сократить код на:

private static PermanentPlacement PopulatePermanentPlacement(InvoiceLine invoiceLine) 
{ 
    var creditReissue = invoiceLine.CreditReissue; 
    while(creditReissue.InvoiceLine.PermanentPlacement == null) 
     creditReissue = creditReissue.InvoiceLine.CreditReissue; 

    return creditReissue.InvoiceLine.PermanentPlacement; 
} 

Это делает то же самое, как ваш код, за исключением того, что он обращается к PermantPlacement окончательного InvoiceLine дополнительное время. Поэтому, если это свойство getter делает больше, чем просто возвращает значение, эта модификация может быть недействительной.

Answer 3

Ваш код кажется довольно понятным и понятным для меня. Ответ Рене Фогта о столь же коротким, как вы могли бы сделать это, не меняя логики, но у меня есть внутреннее чувство, что вы действительно хотите, может быть это:

private static PermanentPlacement PopulatePermanentPlacement(InvoiceLine invoiceLine) 
{ 
    var creditReissue = invoiceLine.CreditReissue; 
    while (creditReissue.InvoiceLine.CreditReissue != null) 
    { 
     // Get the last credit reissue 
     creditReissue = creditReissue.InvoiceLine.CreditReissue; 
    } 
    return creditReissue.InvoiceLine.PermanentPlacement;  
} 

Это делает изменения логики немного, пожалуйста, подтвердите, что оно эквивалентно.