2012-01-31 6 views
4

다른 개체에서 다른 많은 함수를 호출하는 함수가 있습니다. 각 함수는 다음 함수를 호출하기 전에 true를 리턴해야합니다. 보시다시피 너무 많은 if 문을 사용하고 있습니다. 어떻게 코드를 개선하고 더 깔끔하게 만들 수 있습니까? 감사합니다다른 함수를 호출 할 때 if 문을 줄이기

아마도
bool ISOKToDoSomthing() 
{ 
    boo retVal = false; 

    retVal = ObjA.CheckVersion(oldVersion); 

    if(retVal) 
    { 
     retVal = objB.CheckUserRight(); 
    } 

    if(retVal) 
    { 
      retVal = ObjC.ISDBExist(); 
    } 

    if(retVal) 
    { 
      retVal = OjbD.ISServerUp(ServerName); 
    } 
    //tons of similar code as above 
    ......... 
    return retVal; 
} 
+1

이 질문에 대한 많은 변형이 이전에 요청되었습니다. – leppie

답변

9
return 
    ObjA.CheckVersion(oldVersion) && 
    objB.CheckUserRight() && 
    ObjC.ISDBExist() && 
    OjbD.ISServerUp(ServerName); 
5
return ObjA.CheckVersion(oldVersion) && objB.CheckUserRight() && ObjC.ISDBExist() && OjbD.ISServerUp(ServerName) 
2

?

retVal = objB.CheckUserRight() && ObjC.ISDBExist() && OjbD.ISServerUp(ServerName); 

보조 노트 objB는 조건이되지 않았습니다 즉시 실행을 중단합니다 (하나 개의 문장의 코드를 그것에 메서드를 호출하기 전에 null의 경우, 당신은, 예를 들어 테스트 할 수 있습니다 즉, 다음 조건을 호출하지 않습니다.) 그래서 if (objB! = null) 유형 명령문이 많이 필요하지 않습니다. 예 : 당신은 너무처럼 그들을 체인 수

retVal = (objB != null && objB.CheckUserRight()) && (ObjC != null && ObjC.ISDBExist()) && (OjbD != null && OjbD.ISServerUp(ServerName)); 
+1

왜 괄호가 너무 많습니까? –

+0

-1 여분의 괄호가 너무 많습니다. – Polyfun

+0

당신은 절대적으로 옳고 수정되었습니다. 괄호는 조건을 설명하는 데 도움이되지만 조금 더 읽기 쉽지만 이전 버전은 약간 OTT였습니다! – Jeb

2

& & 운영자 것이다 단락 : 그래서,

and 사용에 대한
bool ISOKToDoSomthing() 
{ 
    return 
     ObjA.CheckVersion(string oldVersion) && 
     objB.CheckUserRight() && 
     ObjC.ISDBExist() && 
     OjbD.ISServerUp(ServerName) && 

    //tons of similar code as above 
    ......... 

} 
1

방법 :

retVal = ObjA.CheckVersion(oldVersion) && 
     objB.CheckUserRight() && 
     ObjC.ISDBExist() && 
     OjbD.ISServerUp(ServerName); 
return retval; 
2

당신은 사실을 활용할 수 있습니다 그 C#는 단락 회로 평가를 수행합니다 :

,210

편집은 while 루프를 시도 할 수, 코드가 덜 장황하려면 CheckVersion의 매개 변수

+1

당신은 단지 복사/붙여 넣기를 할 것 같지만,'CheckVersion'을 수정하는 것은 좋은 생각 일 수 있습니다. –

+0

사실, 함수가 수행하는 작업에 너무 많은주의를 기울이지 않고 IF 블록을 줄였습니다. :) –

2
bool ISOKToDoSomthing() 
{ 
    return ObjA.CheckVersion(string oldVersion) && 
      ObjB.CheckUserRight() && 
      ObjC.ISDBExist() && 
      OjbD.ISServerUp(ServerName); 
} 
1

에 구문을 수정합니다. 당신의 메소드가 원래 값의 값을 변경하지 않는다는 것을 감안할 때, (retval) {} 동안 while 액션리스트를 반복합니다. 개인적으로, 나는 이것이 추악하다고 생각합니다. 스위치를 사용해 보거나 비트 열거 형 (enuck)을 사용하는 것을 고려해보십시오.

내 관점에서 볼 때, 이와 같은 코드를 작성하는 것을 보았을 때 나는 어딘가에서 심각한 건축 실수를 저질렀습니다.이 호출을하는 이유에 대해 다시 생각해 봐야합니다. 아마 당신은 당신의 코드가 아닌 당신의 논리를 다시 한번 살펴 봐야 할 것입니다. 앉아서 상자를 그리고 디자인 단계에서 조금 더 일하면서 물건을 아주 다르게 만들 수 있습니다.

편집 : 그렇지 않으면 다른 모든 사람들과 마찬가지로 하나의 if 문을 반복 할 수 있습니다. 다시 말하지만, 이것은 많은 부울 목록보다 큰 문제입니다.

6
if (!ObjA.CheckVersion(oldVersion)) return false; 
if (!ObjB.CheckUserRight()) return false; 
if (!ObjC.IsDBExist()) return false; 
if (!ObjD.IsServerUp(serverName)) return false; 

... your other checks ... 

return true; 

&&의 단락이 몇 가지 조건에 유용하지만, 당신이 그들의 "톤"이있는 경우, IMO 것을 시도하고 하나 개의 문장에 충실 너무 많이 있습니다.

두 가지의 조합이 유용 할 수 있습니다. 더 유용한 것은 여전히 ​​이러한 수표 중 일부를 더 큰 덩어리로 응축하는 것입니다 (그러나 IsOKToDoSomething보다 작음).예를 들어 데이터베이스에 대한 액세스 권한 (존재 여부, 로그인 가능 여부 등)을 한꺼번에 확인하십시오.

진실은 힌트를 확인할 수있는 디자인 문제 - 즉, 한 번에 너무 많은 것을하려고하거나, 시스템의 모든 측면에서 작은 촉수를 가진 "신의 물건"을 가지고 있습니다. 그 문제를 고칠 수도 있습니다.

+0

감사. 아주 좋은 조언. 하지만 그것은 코드의 작은 부분만을 가지고있는 오래된 시스템입니다 – c830

+0

개인적으로 저는 Eric의 제안과 cHao의 제안 사이에서 타협 할 것입니다. 나는 읽기 쉽지만 기존 코드보다 짧기 때문에 cHao의 솔루션을 선호합니다. 한 번에 더 많은 화면 기능을 볼 수 있기를 좋아합니다. 그러나 다음에 ISOKToDoSomthing을 변경해야 할 때까지이 변경 작업을 보류 할 것을 제안합니다. 관련 코드를 적극적으로 사용하는 경우 리팩토링을 저장하십시오. 아무도 작동하지 않는 작업 코드는 리팩터링이 필요하지 않습니다. – Brian

0

변경하려는 정도에 따라 다릅니다. 아마도 하위 메서드에서 bool을 반환하는 대신 예외를 throw 할 수 있습니다. 당신이 뭔가를 할 경우 (IS는 일반적으로에 내 마음에 "부울을 반환"번역 때문에),하지만 당신은 사진을 찍어

bool retVal = true; 

try 
{ 
    ObjA.CheckVersion(oldVersion); 
    objB.CheckUserRight(); 
    ObjC.ISDBExist(); 
    OjbD.ISServerUp(ServerName); 
} 
catch (SomeException ex) 
{ 
    // Log ex here. 
    retVal = false; 
} 

return retVal; 

IsDBExist 아마 가장 좋은 이름이 아닙니다.

8

제 조언 : 변경 작업에 대한 명확한 비즈니스 사례가 없으면이 코드에 아무 것도하지 마십시오.

코드는 분명하고 정확하며 유지 관리가 쉽고 디버깅하기 쉽습니다. 어떤 이유로 지구상을 변경하고 싶습니까?? 불필요하게 작업 코드를 변경하지 않고 버그를 수정하고 기능을 추가하여 시간을 투자하십시오. 당신의 상사가 당신에게 "당신은 오늘 무엇을 했습니까?"라고 묻습니다. 대답은 "나는 수정되고, 작동하고, 이미 디버깅 된 코드를 만들기 위해 불필요한 수정을가함으로써 우리의 일정 위험을 증가시켰다".

여기에 실제로 문제가있는 경우 문제는 코드를 읽기 어렵지 않고 코드가 사용자가 구성 할 수있는 비즈니스 프로세스가되도록 엄격하게 인코딩한다는 것입니다. 이 경우 비즈니스 프로세스를 인코딩하는 "워크 플로"라는 개체를 만들고 임의 워크 플로를 평가하는 엔진을 만듭니다. 그런 다음 사용자의 입력을 기반으로 원하는 워크 플로를 나타내는 해당 개체의 인스턴스를 만듭니다.

실제로 사용자에게 가치를 더합니다. 사용자는 중첩 된 "if"문을 사용하는지 여부를 조금도 신경 쓰지 않습니다.

관련 문제