C# là thiết kế này "đúng"?

Question

Tôi hiện có

 if (!RunCommand(LogonAsAServiceCommand)) 
      return; 

     if (!ServicesRunningOrStart()) 
      return; 

     if (!ServicesStoppedOrHalt()) 
      return; 

     if (!BashCommand(CreateRuntimeBashCommand)) 
      return; 

     if (!ServicesStoppedOrHalt()) 
      return; 

     if (!BashCommand(BootstrapDataBashCommand)) 
      return; 

     if (!ServicesRunningOrStart()) 
      return; 

làm việc này sẽ sạch hơn? nó có an toàn không?

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

Answer 1

Bạn nên tuân theo mọi thứ dễ đọc hơn và dễ hiểu hơn.

Trừ khi thực tế không hiệu quả.

Answer 2

Mã đầu tiên có phải là một bộ câu lệnh OR không?

if ( 
     (!RunCommand(LogonAsAServiceCommand)) 
    || (!ServicesRunningOrStart()) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(CreateRuntimeBashCommand)) 
    || (!ServicesStoppedOrHalt()) 
    || (!BashCommand(BootstrapDataBashCommand)) 
    || (!ServicesRunningOrStart()) 

Answer 3

Hoặc là không sao. Tuy nhiên từ một quan điểm phong cách tôi muốn đề nghị rằng vì đây là một loạt các comands bạn có thể sử dụng một List<Action> để giữ chúng và làm cho dữ liệu chức năng điều khiển.

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

Tất nhiên kể từ khi các chức năng đã có chữ ký bạn có thể phải làm nhiều người ...

một lần nữa đó là một vấn đề phong cách khác nhau ....

Answer 4

Khi tôi nhìn vào cách tiếp cận đầu tiên, suy nghĩ đầu tiên của tôi là lý do mã được viết theo cách đó là các hoạt động liên quan có thể có tác dụng phụ. Từ tên của các phương pháp, tôi giả sử họ làm gì? Nếu vậy, tôi chắc chắn sẽ đi với cách tiếp cận đầu tiên và không phải là thứ hai; Tôi nghĩ rằng người đọc mã của bạn sẽ thấy rất khó hiểu khi thấy biểu thức ngắn mạch && đang được sử dụng để thực thi các điều kiện phụ.

Nếu không có tác dụng phụ, sẽ thực hiện; cả hai đều có thể đọc được hoàn toàn. Nếu bạn thấy rằng có một số điều kiện nhiều hơn hoặc các điều kiện tự khác nhau trong các tình huống khác nhau, bạn có thể thử một cái gì đó như:

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

tôi sẽ không thực sự đi với một cách tiếp cận như vậy trong trường hợp của bạn, tuy nhiên.

Answer 5

Nếu bạn đang sử dụng mã này nhiều lần tôi sẽ đề nghị bạn đặt nó trong một phương pháp riêng biệt như:

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

và gọi nó khi cần thiết

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