Как бы я упростил этот код? - PullRequest
Новая
       15

Как бы я упростил этот код?

3 голосов
/ 25 ноября 2011 
boolean f(boolean A, boolean B, boolean C, boolean D, boolean E)
{
  if (A)
  {
    k();
    if (B)
    {
      m();
      if (C)
      {
        n();
        if (D)
        {
          p();
          if (E)
          {
            q();
            return true;
          }
          else
          {
            r();
            return false;
          }
        }
        else
        {
          s();
          return false;
        }
      }
      else
      {
        t();
        return false;
      }
    }
    else
    {
      v();
      return false;
    }
  }
  else
  {
    w();
    return false;
  }
}

Ответы [ 4 ]

3 голосов
/ 25 ноября 2011

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

if (A) k(); else w();
if (A && B) m(); else if(A && !B) v();
if (A && B && C) n(); else if (A && B && !C) t();
if (A && B && C && D) p(); else if (A && B && C && !D) s();
if (A && B && C && D && E) q(); else if (A && B && C && D && !E) r();

return (A && B && C && D && E);
3 голосов
/ 25 ноября 2011

Не зная больше о проблеме, которую вы решаете, я бы переписал ее как 

boolean f(boolean A, boolean B, boolean C, boolean D, boolean E)
{
  if (A) k();
  if (A && B) m();
  if (A && B && C) n();
  if (A && B && C && D) p();
  if (A && B && C && D && E) { q(); return true; }
  if (A && B && C && D && !E) { r(); return false; }
  if (A && B && C && !D) { s(); return false; }
  if (A && B && !C) { t(); return false; }
  if (A && !B) { v(); return false; }
  if (!A) { w(); return false; }
}

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

0 голосов
/ 07 декабря 2012

Меня попросили оптимизировать этот код во время недавнего собеседования.

Вот версия кода, которую я придумал: 

boolean f(boolean A, boolean B, boolean C, boolean D, boolean E )
{
    boolean success = false;

    // I use "==false" because it's more readable than "if !A"
    if(A == false)
    {
        w();
    } else {
        if(B == false)
        {
            v();
        } else {
            m();
            if(C == false)
            {
                n();
                if(D == false)
                {
                    s();
                } else {
                    if(E == false)
                    {
                        r();
                    } else {
                        q();
                        success = true;
                    }
                }
            }
        }
    }

    // this will be "false" in all cases except one
    return (success);
}

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

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

boolean f(Boolean A, Boolean B, Boolean C, Boolean D, Boolean E)
{
    boolean result = false;

    do
    {
        if (!A)
        {
            w();
            break;
        }

        k();
        if (!B)
        {
            v();
            break;
        }

        m();
        if (!C)
        {
            t();
            break;
        }

        n();
        if (!D)
        {
            s();
            break;
        }

        p();
        if (!E)
        {
            r();
            break;
        }

        // All conditions satisfied
        result = true;

    } while (false);

    return result;
}

Здесь используется хитрый цикл «один раз и только один раз» с идеей «break»"- когда не удается выполнить какое-либо условие.

0 голосов
/ 25 ноября 2011

Если порядок вызова методов не важен, то: 

failedOnce = false
for ar as Array in [
    (A, K, W)
    (B, M, V)
    (C, N, T)
    (D, P, S)
    (E, Q, R)
    ]:
    if ar[0]:
        ar[1].Invoke()
    else:
        ar[2].Invoke()
        break
        failedOnce = false

return not failedOnce
...