물론 코드 내부를 봐야겠지만... 개인적으로는 위의 코드가 더 읽기 좋은 것 같네요.

난 아래와 같이 리팩토링하는 것에 반대하고 싶다. 위 코드의 경우는 특히더 그렇다고 생각한다. 먼저 정상 로직을 실행하기 전에 예외 상황을 확인하고 이에 대한 처리를 하는 것이 버그를 줄일 수 있는 안전한 프로그래밍이라 생각한다. 그런 관점에서 나라면 아래와 같이 리팩토링할 듯하다.

if(checkAllFail()) {
  allCancel();
  return buildAllFailMessage();
}


return buildPartFailMessage();

정상 로직에 대해 else를 사용할 필요는 없다고 판단된다. 그리고 위와 같은 코드가 메소드 가운데에 있을 경우 바로 return하기 힘들다면 그 부분만 Extract Method 리팩토링을 통해 분리한 후 위와 같이 구현할 수 있으리라 판단된다.

질문 코드에서 아래와 같이 리팩토링할 경우 이 소스 코드를 읽어야 되는 개발자는 if 절의 조건절을 두번씩이 확인하고 차이가 있는지도 판단해야 하기 때문에 그리 좋은 코드는 아니라고 생각한다.

@자바지기 해당 블록 자체를 리팩토링 하는 것으로 관점을 더 올렸군요. 그래 그래.. 그랬어야해.. ^^

그냥 규모가 좀 있는 코드라고 해서 조큼 오버 한다면..^^ interface로 postProcesss() 하나 정의하고 두개의 구현체 - 전체 실패, 부분 실패 -를 생성해서 각 로직에 맞게 선택해서 사용? 할 것 같아요~ (오버죠~??)

@newoverguy 뭐 규모가 좀 큰 경우에는 가능하다고 생각한다. 아마도 그 경우에는 success와 fail에 대한 구현부가 복잡해 진다면 먼저 extract method 리팩토링을 통해 메소드로 분리하는 작업이 선행되어야 할테고, 이후에도 복잡도가 높아지고, 클래스에 역할이 많아진다면 별도의 클래스로 분리한 후에 호출하도록 구현하는 것이 맞을 듯하다.

프로그래밍에 정답이 없는 것이 현재 프로그래밍의 context에 따라 달라지는 것이니 각 상황에 맞춰서 그에 맞는 프로그래밍을 할 수 있는 능력을 길러야 겠지. 근데 그게 힘들단 말이지.

문맥을 추측해보니 실패상황에 대한 처리인 것 같은데요.

전체실패의 경우 전체취소 처리를 하고 (핵심 로직부) 메시지에 대한 건 별도 메소드에서 처리해보았습니다.

실패상황에 따라 message 내용이 다양해진다면, message 내용을 Enum으로 만들어서 실패유형에 따라 return 하는건 어떨까요?

답변들처럼 상황에 따라 많이 달라질듯하네요.ㅎㅎ

```boolean isAllFail = checkAllFail(); if (isAllFail) { allCancel(); } message = buildMessage(isAllFail);

private String buildMessage(boolean isAllFail) { if (isAllFail) return buildAllFailMessage(); else return buildPartFailMessage(); }```

checkAllFail()가 light하다면, 그냥 inline temp (임시변수를 바로 메소드 호출로 변경) 해도 될 것 같습니다.

@lark @newoverguy 현재 상황으로는 과도한 모듈 분리인 것 같은 느낌이네요. if를 많이 써서 좋을 건 없다고 봐서요. 특히나 파라미터로 분기플레그를 넘기는 건 없애는 방향으로 ... 상태값이 먼 곳에도 영향을 주는 건 장기적으로 유지보수 복잡도를 높이는데 크게 기여하는 것 같아요.

아래 패턴이 좋다고 봅니다. 한 번의 분기코드 + 순차적으로 읽어갈 수 있는 코드

@lark 님께서 제 의도를 파악하신듯 사실 위 코드에서 아래 부분은 의도가 있는 if 절의 분리였습니다. ^^ 말씀들하신데로 상황에 따라 달라지겠는데.. 최종적으로는 message를 만드는 코드를 extract method 하고 싶었던거죠

약간은 병적(?)으로 코드상에서 대칭성을 부여하다보니 extract method 할 수 있는 포인트가 나왔다고 생각합니다.