C # этот дизайн "Правильно"? - PullRequest
6 голосов
/ 15 октября 2010

В настоящее время у меня есть следующее

        if (!RunCommand(LogonAsAServiceCommand))
            return;

        if (!ServicesRunningOrStart())
            return;

        if (!ServicesStoppedOrHalt())
            return;

        if (!BashCommand(CreateRuntimeBashCommand))
            return;

        if (!ServicesStoppedOrHalt())
            return;

        if (!BashCommand(BootstrapDataBashCommand))
            return;

        if (!ServicesRunningOrStart())
            return;

Было бы чище сделать это?это безопасно?

        if (
           (RunCommand(LogonAsAServiceCommand))
        && (ServicesRunningOrStart())
        && (ServicesStoppedOrHalt())
        && (BashCommand(CreateRuntimeBashCommand))
        && (ServicesStoppedOrHalt())
        && (BashCommand(BootstrapDataBashCommand))
        && (ServicesRunningOrStart())
        )
        {
               // code after "return statements" here
        }

Ответы [ 5 ]

4 голосов
/ 15 октября 2010

Когда я смотрю на первый подход, моя первая мысль состоит в том, что причина, по которой код был написан таким образом, заключается в том, что соответствующие операции, вероятно, имеют побочные эффекты. Из названий методов я предполагаю, что они делают? Если это так, я бы определенно выбрал первый подход, а не второй; Думаю, читателям вашего кода было бы очень странно видеть короткое замыкание &&, используемое для условного выполнения побочных эффектов.

Если побочных эффектов нет, подойдет любой; они оба отлично читаются. Если вы обнаружите, что есть еще несколько условий или сами условия различаются в разных сценариях, вы можете попробовать что-то вроде:

Func<bool>[] conditions = ...
if(conditions.Any(condn => condn()))
{
   ...
}

Однако я бы не стал использовать такой подход в вашем случае.

3 голосов
/ 15 октября 2010

Является ли первый код набором операторов OR?

if ( 
       (!RunCommand(LogonAsAServiceCommand)) 
    || (!ServicesRunningOrStart()) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(CreateRuntimeBashCommand)) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(BootstrapDataBashCommand)) 
    || (!ServicesRunningOrStart()) 
2 голосов
/ 15 октября 2010

Либо в порядке. Однако, с точки зрения стиля, я бы предположил, что, поскольку это серия команд, вы можете использовать List<Action> для их хранения и управления данными функции.

(new Action[] {  func1, func2, .... }).ToList().ForEach(a => a());

Конечно, поскольку функции имеют разные подписи, вам, возможно, придется сделать несколько подписей ...

опять же это вопрос стиля ....

1 голос
/ 15 октября 2010

Вы должны придерживаться того, что более читабельно и понятно.

Если это действительно неэффективно.

0 голосов
/ 15 октября 2010

Если вы используете этот код несколько раз, я бы посоветовал вам поместить его в отдельный метод, например:

public static class HelperClass
{
  public static bool ServicesRunning()
  {
    // Your code here... 
  }
}

и вызывать его при необходимости

public class SomeClass
{
  // Constructors etc...
  public void SomeMethod()
  {
     if(HelperClass.ServicesRunning())
     {
        // Your code here...
     }
  }
}
...