2013-02-22 40 views
5

Ổi cung cấp chức năng trợ giúp để kiểm tra các điều kiện tiên quyết nhưng tôi không thể tìm thấy các hàm trợ giúp để kiểm tra kết quả trung gian.Tôi có nên sử dụng kiểm tra điều kiện tiên quyết để kiểm tra kết quả trung gian không?

private void foo(String param) 
{ 
    checkNotNull(param, "Required parameter is not set"); 

    int x = get(param); 
    if (x == -1) { 
     throw new RuntimeException("This should never have happened and indicates a bug."); 
    } 
} 
  • Tôi có nên quấn if (...) {....} gia helper của riêng tôi?
  • Hoặc tôi có nên sử dụng checkState từ Ổi?
  • Hoặc tôi có nên xem lỗi của get() do hậu quả của param và sử dụng checkArgument không?
  • Tôi có nên sử dụng các xác nhận trong những trường hợp này không?
  • Hoặc tôi có thiếu gì đó không?

Cảm ơn bạn đã cân nhắc.

+1

Cách mã của bạn kiểm tra dữ liệu là hợp lệ, tuy nhiên bạn có thể muốn sử dụng ngoại lệ 'cụ thể' hơn 'RuntimeException' - có lẽ là' IllegalStateException' hoặc một trong số của riêng bạn. – vikingsteve

+2

Thông thường, một * "Điều này không bao giờ nên xảy ra" * ngoại lệ phải là một AssertionError, chứ không phải là một RuntimeException. Sử dụng khẳng định là một chút mong manh vì nó dựa trên các đối số JVM ... Ngoài ra mã của bạn có vẻ ổn với tôi. – assylias

+1

Có vẻ như nhận được -1 là kết quả của việc làm việc với đối số là một 'IllegalArgumentException' hợp lệ, do đó tôi sẽ sử dụng' checkArgument (x! -1, "message của tôi") ' –

Trả lời

7

Đó là một nơi nào đó giữa một vấn đề ưu tiên và một quy ước.

Nói chung, mọi người sẽ sử dụng các xác nhận để chỉ ra lỗi lập trình; tức là, "nếu tôi đã làm đúng công việc của mình, thì param không phải rỗng sẽ không bao giờ dẫn đến -1 từ get, bất kể đầu vào của người dùng hoặc các lực lượng bên ngoài khác." Tôi đối xử với họ gần như là ý kiến ​​mà có thể tùy chọn được xác minh trong thời gian chạy. Mặt khác, nếu get có thể trả về -1 trong một số trường hợp, nhưng đầu vào đó không hợp lệ, thì tôi thường ném IllegalArgumentExceptioncheckArgument là một cách hoàn toàn hợp lý để thực hiện việc này. Một nhược điểm này là khi bạn bắt được nó, nó có thể đến từ khá nhiều nơi. Xem xét:

try { 
    baz(); 
    bar(); 
    foo(myInput); 
} catch (IllegalArgumentException e) { 
    // Where did this come from!? 
    // It could have come from foo(myInput), or baz(), or bar(), 
    // or some method that any of them invoked, or really anywhere 
    // in that stack. 
    // It could be something totally unrelated to user input, just 
    // a bug somewhere in my code. 
    // Handle it somehow... 
} 

Trong trường hợp có vấn đề - ví dụ, bạn muốn bật lên một lưu ý rất hữu ích cho người dùng rằng họ đang không được phép vào -1 ở dạng đầu vào của họ - bạn có thể muốn ném một ngoại lệ tùy chỉnh để bạn có thể dễ dàng nắm bắt nó sau này:

try { 
    baz(); 
    bar(); 
    foo(myInput); 
} catch (BadUserInputException e) { 
    reportError("Bad input: " + e.getMessage()); 
    log.info("recorded bad user input", e); 
} 

Vì đối với checkState, nó thực sự không đúng với tôi. Ngoại lệ đó thường ngụ ý rằng vấn đề là trạng thái mà this ở trong (hoặc một số trạng thái toàn cầu khác trong ứng dụng). Từ the docs:

Tín hiệu đã được gọi vào một thời điểm bất hợp pháp hoặc không phù hợp.

Trong trường hợp của bạn, -1 là không bao giờ phù hợp, vì vậy checkState là gây hiểu lầm.Bây giờ, nếu nó đã là:

if (x == -1 && (!allowNegativeOne()) { ... } 

... thì điều đó sẽ phù hợp hơn, mặc dù vẫn có nhược điểm là IllegalArgumentException ở trên.

Vì vậy, cuối cùng, có câu hỏi liệu bạn chỉ nên giữ if như cũ hoặc sử dụng phương thức trợ giúp. Điều đó thực sự làm giảm hương vị, mức độ phức tạp của séc và mức độ thường xuyên được sử dụng (ví dụ: trong các phương pháp khác). Nếu kiểm tra đơn giản như x == -1 và kiểm tra đó không bao giờ được thực hiện bằng các phương pháp khác (vì vậy việc sử dụng lại mã không phải là vấn đề), tôi sẽ chỉ giữ if.

2

Nếu phương pháp get chỉ đơn giản là chuyển đổi chuỗi thành một int, sau đó nó nên làm xác thực ở đó, tốt nhất là ném một exceptionArgumentException hoặc một số RuntimeException như vậy. Với ở trên, bạn cũng đang trộn các mức trừu tượng trong phương thức của bạn. Ví dụ. checkNotNull tóm tắt kiểm tra của param cho null, nhưng việc kiểm tra cho param dưới dạng int được chia cho phương pháp get và phương pháp foo. Tại sao không có một phương thức loại checkPreCondition? Ví dụ.

private void paramShouldBeNonNullInt(String value) { 
    if (value == null) throw new IllegalArgumentException("value was null"); 
    try { 
     Integer.parseInt(value) 
    } catch (NumberFormatException e) { 
     throw new IllegalArgumentException("value was not an integer"); 
    } 
} 
+0

' get' chỉ là một ví dụ, xin vui lòng bỏ qua ngữ nghĩa, cũng 'checkNotNull' là có một ví dụ về những gì tôi muốn phần' if (...) 'trông giống như, về bản chất tôi chỉ thiếu một' checkOrThrowRuntime() 'chức năng hoặc có thể' checkState' là thứ tôi thực sự muốn. –

2

Trước hết, bạn cần phân biệt giữa các hợp đồng (ví dụ: xác nhận/lỗi lập trình) và xử lý lỗi (ví dụ: các ngoại lệ có thể khôi phục và có thể bị bắt và khôi phục).

Nếu bạn cần kiểm tra kết quả trung gian, có vẻ như bạn không tin tưởng dịch vụ được gọi và bạn muốn đảm bảo các giả định của bạn được giữ. Đúng? Điều này nên được thể hiện như một khẳng định, và ổi không có hỗ trợ rất tốt cho điều đó.

Hãy xem valid4j. Đã tìm thấy ở đây https://github.com/helsing/valid4j và tại đây http://www.valid4j.org.

tôi sẽ sau đó đã bày tỏ các mã như thế này (sử dụng hỗ trợ valid4j cho hamcrest-quẹt):

private int getFoo(String param) { 
    require(param, notNullValue()); // Violation means programming error at client 

    int x = get(param); 

    ensure(x, not(equalTo(-1)); // Violation means programming error at supplier 
    return x; 
} 
0

Một số câu trả lời xuất sắc khác ở đây.

Từ điều kiện tiên quyết javadoc:

ngoại lệ kiện tiên quyết được dùng để báo hiệu rằng gọi phương thức đã mắc phải lỗi. (...) Postcondition hoặc thất bại bất biến khác không nên ném các loại trường hợp ngoại lệ.

Vậy ...

Tôi có nên quấn if (...) {....} gia helper của riêng tôi?

Không, các cơ sở hiện có phải đủ tốt.

Hoặc tôi có nên sử dụng checkState từ Ổi?

Có thể: nếu thông số cần được tải từ tệp trước khi phương thức này được gọi, thì đó sẽ là một phần của hợp đồng về cách sử dụng lớp này.

Hoặc tôi có nên xem lỗi của get() do hậu quả của tham số và sử dụng checkArgument không?

Có thể: ví dụ: nếu có một số hạn chế định dạng trên cú pháp của tham số. (Mặc dù có lẽ sẽ đi vào bên trong get())

Tôi có nên sử dụng xác nhận trong những trường hợp này không?

Có. Nếu nó không phải là một điều kiện tiên quyết như trên, thì thông thường tôi chỉ sử dụng assert ở đây. Đừng quên bạn vẫn có thể thêm thông báo:

assert x != 1 : "Indicates a bug."; 

Tôi thấy điều này phù hợp với mong đợi tài liệu và xác minh việc thực hiện nội bộ/riêng tư của lớp học hoặc phương pháp.

Nếu bạn muốn thực hiện kiểm tra thời gian chạy, bạn có thể làm if (...) throw AssertionError nhưng điều đó có thể chỉ cần thiết nếu bạn đang làm việc với mã không tốt mà bạn không tin tưởng.

Các vấn đề liên quan