Đó 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 IllegalArgumentException
và checkArgument
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
.
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
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
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") ' –