ifネストされたステートメントが多すぎるメソッドを書き直したい。
私はこのアプローチを思いつき、あなたの意見を求めました：
public void MyMethod()
{
bool hasFailed = false;
try
{
GetNewOrders(out hasFailed);
if(!hasFailed)
CheckInventory(out hasFailed);
if(!hasFailed)
PreOrder(out hasFailed);
// etc
}
catch(Exception ex)
{
}
finally
{
if(hasFailed)
{
// do something
}
}
}
私はそれに似たようなことをしましたが、例外処理はしていません:
BOOL ok = CallSomeFunction();
if( ok ) ok = CallSomeOtherFunction();
if( ok ) ok = CallYetAnotherFunction();
if( ok ) ok = WowThatsALotOfFunctions();
if( !ok ) {
// handle failure
}
または、賢くしたい場合：
BOOL ok = CallSomeFunction();
ok &= CallSomeOtherFunction();
ok &= CallYetAnotherFunction();
...
とにかく例外を使用している場合、なぜhasFailed変数が必要なのですか?
あまり。「catch」ブロックでキャッチされるエラーの場合、メソッドは例外を発生させる必要があります。
私が見る限り、これはカスケード ステップの例であり、1 番目と 1 番目と 2 番目が有効な場合に 2 番目と 3 番目のステップが実行されます。つまり、hasFailed==false を返します。
このコードは、Template Method と Decorator デザイン パターンを使用して、より洗練されたものにすることができます。
1 つのインターフェイス、具体的な実装、抽象クラス、および抽象クラスのいくつかのサブクラスが必要です。
public interface Validator {
public boolean isValid();
}
public class GetNewOrders implements Validator {
public boolean isValid() {
// same code as your GetNewOrders method
}
}
public abstract class AbstractValidator implements Validator {
private final Validator validator;
public AbstractValidator(Validator validator) {
this.validator = validator;
}
protected boolean predicate();
protected boolean isInvalid();
public final boolean isValid() {
if (!this.validator.isValid() && predicate() && isInvalid())
return false;
return true;
}
}
public class CheckInventory extends AbstractValidator {
public CheckInventory(Validator validator) {
super(validator);
}
@Override
public boolean predicate() {
return true;
}
@Override
public boolean isInvalid() {
// same code as your CheckInventory method
}
}
public class PreOrder extends AbstractValidator {
public CheckInventory(Validator validator) {
super(validator);
}
@Override
public boolean predicate() {
return true;
}
@Override
public boolean isInvalid() {
// same code as your PreOrder method
}
}
これで、メソッドはより洗練されたものになります。
public void MyMethod() {
bool success = false;
try {
Validator validator = new GetNewOrders();
validator = new CheckInventory(validator);
validator = new PreOrder(validator);
success = validator.isValid();
} finally {
if (!success) {
// do something
}
}
}
Validator オブジェクトは 1 行で作成できますが、検証の順序が明確になるため、このスタイルを好みます。チェーン内に新しい検証リンクを作成するには、AbstractValidator クラスをサブクラス化し、predicate および isInvalid メソッドを実装する必要があります。
そこで何が起こっているのか本当にわからないので、try/catch についてコメントせずに、呼び出されたメソッドが成功のために true/false を返すように変更し、ブール値の短絡に応じてそれらをチェックして、呼び出しを回避します。前の方法が失敗した場合は、後の方法。
public void MyMethod()
{
bool success = false;
try
{
success = GetNewOrders()
&& CheckInventory()
&& PreOrder();
// etc
}
catch(Exception ex) { }
finally
{
if(!success)
{
}
}
}
これは私にはよく見えません。hasFailed 変数の使用は、実際には良くありません。GetNewOrders が例外で失敗した場合、たとえば、 hasFailed = false で catch ブロック内に行き着きます!
ここでの他の回答とは対照的に、例外ではないブール値「hasFailed」の正当な使用があるかもしれないと私は信じています。しかし、そのような条件を例外ハンドラーに混在させるべきではないと思います。
注文のリストを取得して処理し、エラーが発生したときに処理を停止するように見えるコードは好きではありません。確かにその注文をスキップして次の注文に移動する必要がありますか？完全に失敗する唯一のことは、データベース（注文のソース、事前注文の宛先）が停止したときです。ロジック全体が少しファンキーだと思いますが、それはあなたが使っている言語の経験がないからかもしれません。
try {
// Get all of the orders here
// Either in bulk, or just a list of the new order ids that you'll call the DB
// each time for, i'll use the former for clarity.
List<Order> orders = getNewOrders();
// If no new orders, we will cry a little and look for a new job
if (orders != null && orders.size() > 0) {
for (Order o : orders) {
try {
for (OrderItem i : o.getOrderItems()) {
if (checkInventory(i)) {
// Reserve that item for this order
preOrder(o, i);
} else {
// Out of stock, call the magic out of stock function
failOrderItem(o, i);
}
}
} catch (OrderProcessingException ope) {
// log error and flag this order as requiring attention and
// other things relating to order processing errors that aren't database related
}
}
} else {
shedTears();
}
} catch (SQLException e) {
// Database Error, log and flag to developers for investigation
}
私はおそらくいくつかの投稿を複製することを知っています: 何が問題なのelseですか? 遅延評価 ( a() && b()) を使用してメソッドをリンクすることもできますが、それは戻り値として与えられるステータスに依存しているため、とにかく読みやすいです。
プログラムの障害が発生した場合、またはプログラムが操作のために例外的な状態になった場合に例外を発生させる必要があるため、例外を発生させる必要があるという投稿者には同意しません。例外はビジネス ロジックではありません。
私は次のようにします：
public void MyMethod()
{
bool success = false;
try
{
GetNewOrders(); // throw GetNewOrdersFailedException
CheckInventory(); // throw CheckInventoryFailedException
PreOrder(); // throw PreOrderFailedException
success = true;
}
catch( GetNewOrdersFailedException e)
{
// Fix it or rollback
}
catch( CheckInventoryFailedException e)
{
// Fix it or rollback
}
catch( PreOrderFailedException e)
{
// Fix it or rollback
}
finally
{
//release resources;
}
}
例外の拡張はかなり簡単です。
public NewExecption : BaseExceptionType {}
クリスのソリューションが最も正しいです。しかし、必要以上のことはすべきではないと思います。ソリューションは拡張可能でなければならず、それで十分です。
だから今はもっとエレガントです:)
public void MyMethod()
{
try
{
GetNewOrders();
CheckInventory();
PreOrder();
// etc
}
finally
{
// do something
}
}
あなたの新しいアプローチは、単純な一連の指示にはそれほど悪くありませんが、追加の手順が追加されるとどうなりますか? トランザクション動作を必要としますか? (PreOrder が失敗した場合はどうなりますか? または PreOrder 後の次のステップが失敗した場合は?)
今後は、コマンド パターンを使用します: http://en.wikipedia.org/wiki/Command_pattern
...そして、各アクションを Execute() と Undo() を実装する具体的なコマンドとしてカプセル化します。
Then it's just a matter of creating a queue of commands and looping until failure or an empty queue. If any step fails, then simply stop and execute Undo() in order on the previous commands. Easy.