역 참조 된 모든 단일 포인터를 널 보호하는 것이 합리적입니까? 멤버를 실수로 삭제 한 NULL후

새로운 직업에서, 나는 다음과 같은 코드에 대한 코드 리뷰에 플래그를 붙였습니다.

PowerManager::PowerManager(IMsgSender* msgSender)
  : msgSender_(msgSender) { }

void PowerManager::SignalShutdown()
{
    msgSender_->sendMsg("shutdown()");
}

마지막 방법은 다음과 같아야한다고 들었습니다.

void PowerManager::SignalShutdown()
{
    if (msgSender_) {
        msgSender_->sendMsg("shutdown()");
    }
}

즉, 내가 해야한다NULL주위에 가드 msgSender_가 개인 데이터 멤버 인 경우에도 변수입니다. 내가이 ‘지혜’에 대해 어떻게 느끼는지 설명하기 위해 expletives를 사용하는 것을 자제하는 것은 어렵다. 설명을 요청하면 몇 년 동안 일부 주니어 프로그래머가 클래스가 어떻게 작동해야하는지에 대해 혼란스러워하고 자신이 없어야 할 멤버를 실수로 삭제 한 NULL후 (그리고 나중에 설정해야 함) 무서운 이야기를 얻습니다. 제품 출시 직후 현장에서 문제가 발생했으며 모든 것을NULL 확인하는 것이 더 낫다는 “어려운 방법을 배우고 우리를 믿습니다” .

나에게 이것은 화물 컬트 프로그래밍 처럼 느껴지고 단순하고 단순합니다. 선의의 동료 몇 명이 저에게 ‘얻기’를 돕고 이것이 더 강력한 코드를 작성하는 데 어떻게 도움이되는지 알아 보려고 노력하고 있습니다. 그러나 나는 그들이 그것을 얻지 못하는 사람이라는 느낌을 줄 수 없습니다. .

코딩 표준 에서 함수에서 역 참조 된 모든 단일 포인터가 NULL개인 데이터 멤버까지도 먼저 확인 하도록 요구하는 것이 합리적 입니까? (참고 : 상황을 설명하기 위해 항공 교통 관제 시스템이나 기타 ‘실패-사람-사람’제품이 아닌 가전 제품을 만듭니다.)

편집 : 위의 예에서 msgSender_공동 작업자는 선택 사항이 아닙니다. 항상 NULL이면 버그를 나타냅니다. 생성자로 전달되는 유일한 이유 PowerManager는 모의 IMsgSender서브 클래스 로 테스트 할 수 있기 때문 입니다.

요약 :이 질문에 정말 좋은 답변이있었습니다. 모두 감사합니다. 나는 @aaronps에서 주로 간결함으로 인해 하나를 수락했습니다. 다음과 같은 상당히 일반적인 계약이있는 것 같습니다.

  1. 의무화 NULL에 대한 경비를 역 참조 포인터 것은 과잉이지만,
  2. 참조 (가능한 경우) 또는 const포인터 를 사용하여 전체 토론을 회피 할 수 있습니다.
  3. assert명령문은 NULL기능의 전제 조건이 충족되는지 확인하기 위해 경비원에 대한 보다 계몽 된 대안 입니다.


답변

‘계약’에 따라 다릅니다.

PowerManager 반드시 유효한 경우 반드시IMsgSender null을 확인하지 말고 더 빨리 죽도록하십시오.

다른 한편으로,이 경우 수도 을 가지고 IMsgSender, 당신은 그 한 간단하게, 당신은 사용할 때마다 확인해야합니다.

후배 프로그래머의 이야기에 대한 최종 의견, 문제는 실제로 테스트 절차의 부족입니다.


답변

코드를 읽어야한다고 생각합니다.

PowerManager::PowerManager(IMsgSender* msgSender)
  : msgSender_(msgSender)
{
    assert(msgSender);
}

void PowerManager::SignalShutdown()
{
    assert(msgSender_);
    msgSender_->sendMsg("shutdown()");
}

이것은 NULL을 지키는 것보다 실제로 낫습니다. NULL 인 경우 함수를 호출해서는 안된다는 것이 매우 분명하기 때문 msgSender_입니다. 또한 이런 일이 발생하면 알 수 있습니다.

동료의 공유 된 “지혜”는이 오류를 자동으로 무시하고 예기치 않은 결과를 초래합니다.

일반적으로 버그는 원인에 더 가깝게 감지되면 수정하기가 더 쉽습니다. 이 예에서 제안 된 NULL 보호는 종료 메시지가 설정되지 않아 눈에 띄는 버그가 발생할 수 있습니다. SignalShutdown전체 응용 프로그램이 방금 사망했을 때보 다 함수를 역으로 처리하는 데 어려움을 겪을 수 SignalShutdown()있습니다.

약간 반 직관적이지만 문제가 발생하자마자 충돌하면 코드가 더 강력 해집니다. 실제로 문제를 발견 하고 매우 명백한 원인이있는 경향이 있기 때문 입니다.


답변

msgSender이 경우 할 수 없다 null, 당신은 놓아야 null만 생성자에서 확인. 이것은 클래스에 대한 다른 입력의 경우에도 마찬가지입니다- ‘모듈’에 들어가는 시점에서 무결성 검사를하십시오-클래스, 함수 등

내가 경험하는 규칙은 모듈 경계 (이 경우 클래스) 사이에서 무결성 검사를 수행하는 것입니다. 또한 클래스는 클래스 멤버의 수명의 무결성을 신속하게 정신적으로 확인할 수있을 정도로 작아야합니다. 따라서 부적절한 삭제 / 널 지정과 같은 실수를 방지 할 수 있습니다. 게시물의 코드에서 수행되는 null 검사는 유효하지 않은 사용이 실제로 포인터에 null을 할당한다고 가정하지만 항상 그런 것은 아닙니다. ‘유효하지 않은 사용’은 본질적으로 일반 코드에 대한 가정이 적용되지 않음을 의미하므로 모든 유형의 포인터 오류 (예 : 잘못된 삭제, 증분 등)를 포착 할 수는 없습니다.

또한 인수가 널이 될 수 없다고 확신하는 경우 클래스 사용에 따라 참조 사용을 고려하십시오. 그렇지 않으면, 원시 포인터 대신 std::unique_ptr또는 std::shared_ptr원시 포인터를 사용하십시오.


답변

아니요, 포인터가 가리키는 모든 포인터 역 참조 를 확인하는 것은 합리적이지 않습니다 NULL.

널 포인터 검사는 선택적 인수가 제공되지 않은 경우 전제 조건이 충족되는지 확인하거나 적절한 조치를 취하기 위해 함수 인수 (생성자 인수 포함)에 중요하며 클래스의 내부를 노출 한 후 클래스의 불변성을 확인하는 것이 좋습니다. 그러나 포인터가 NULL이 된 유일한 이유가 버그가 존재한다면 검사 할 필요가 없습니다. 이 버그는 포인터를 다른 유효하지 않은 값으로 쉽게 설정할 수 있습니다.

내가 당신과 같은 상황에 직면했다면 나는 두 가지 질문을 할 것입니다.

  • 왜 주니어 프로그래머의 실수가 출시 전에 잡히지 않았습니까? 예를 들어 코드 검토 또는 테스트 단계에서? 결함이있는 가전 제품을 시장에 출시하는 것은 안전에 중요한 응용 분야에서 사용되는 결함이있는 장치를 출시하는 것만 큼 비용이 많이들 수 있습니다 (시장 점유율과 영업권을 고려하는 경우). 다른 QA 활동.
  • null 검사가 실패하면 어떤 오류 처리를 수행 할 것으로 예상 assert(msgSender_)됩니까 , 아니면 null 검사 대신 쓸 수 있습니까? 널 (null) 체크인 만 입력하면 충돌이 발생하지 않았지만 실제로 작업이 생략 된 상태에서 작업이 수행되었다는 전제하에 소프트웨어가 계속해서 더 나쁜 상황을 만들었을 수 있습니다. 이로 인해 소프트웨어의 다른 부분이 불안정해질 수 있습니다.

답변

이 예제는 입력 매개 변수가 null †인지 여부보다 개체 수명에 관한 것 같습니다. 당신 PowerManager항상 유효한을 가져야 한다고 언급 했으므로 IMsgSender포인터로 인수를 전달하면 (널 포인터 가능성을 허용) 디자인 결함 ††으로 나에게 충격을줍니다.

이와 같은 상황에서는 호출자의 요구 사항이 언어로 적용되도록 인터페이스를 변경하는 것이 좋습니다.

PowerManager::PowerManager(const IMsgSender& msgSender)
  : msgSender_(msgSender) {}

void PowerManager::SignalShutdown() {
    msgSender_->sendMsg("shutdown()");
}

이 방법으로 다시 작성하면 전체 수명 PowerManagerIMsgSender대한 참조가 필요합니다. 이는 또한 IMsgSender보다 오래 살아야 한다는 암시 적 요구 사항을 설정하고 PowerManager내부의 널 포인터 검사 또는 어설 션의 필요성을 무시합니다 PowerManager.

스마트 포인터 (boost 또는 c ++ 11 사용)를 사용하여 동일한 내용을 작성하여 명시 적으로 IMsgSender보다 오래 살도록 할 수도 있습니다 PowerManager.

PowerManager::PowerManager(std::shared_ptr<IMsgSender> msgSender)
  : msgSender_(msgSender) {}

void PowerManager::SignalShutdown() {
    // Here, we own a smart pointer to IMsgSender, so even if the caller
    // destroys the original pointer, we still have a valid copy
    msgSender_->sendMsg("shutdown()");
}

IMsgSender수명이 수명보다 길어질 수없는 경우 PowerManager(예 :)이 방법을 선호합니다 x = new IMsgSender(); p = new PowerManager(*x);.

† 포인터와 관련하여 : 널 (null) 널 검사는 코드를 읽기 어렵게하고 안정성을 향상시키지 않습니다 ( 안정성 모양 을 개선하여 훨씬 나쁩니다).

어딘가에 누군가가 메모리를위한 주소를 가졌다 IMsgSender. std::bad_alloc유효하지 않은 포인터를 전달하지 않도록 할당이 성공했는지 (라이브러리 리턴 값 확인 또는 예외 처리 )를 확인하는 것은 해당 함수의 책임 입니다.

PowerManager의 소유가 아니기 때문에 IMsgSender(잠시 동안 빌리는 것), 그 메모리를 할당하거나 파기 할 책임이 없습니다. 이것이 내가 참조를 선호하는 또 다른 이유입니다.

††이 직업을 처음 사용했기 때문에 기존 코드를 해킹 할 것으로 예상됩니다. 따라서 디자인 결함으로 인해 결함이 작업중 인 코드에 있음을 의미합니다. 따라서 null 포인터를 확인하지 않기 때문에 코드에 플래그를 지정하는 사람들은 실제로 포인터가 필요한 코드 작성에 대한 플래그를 지정하고 있습니다. 🙂


답변

예외와 마찬가지로 보호 조건은 오류에서 복구 할 작업을 알고 있거나보다 의미있는 예외 메시지를 제공하려는 경우에만 유용합니다.

오류를 삼키는 것 (예외 또는 가드 검사로 잡 히든)은 오류가 중요하지 않은 경우에만 수행해야합니다. 오류가 발생하는 것을 볼 수있는 가장 일반적인 장소는 오류 로깅 코드입니다. 상태 메시지를 기록 할 수 없어서 앱을 중단하고 싶지 않습니다.

함수가 호출 될 때마다 그리고 선택적인 동작이 아니며, 자동으로 실패하지 않고 크게 실패해야합니다.

편집하다: 주니어 프로그래머 이야기에 대해 생각할 때 개인 구성원이 절대로 허용되지 않았을 때 null로 설정되었다는 것이 들렸습니다. 부적절한 글쓰기에 문제가 있었으며 읽을 때 유효성을 검사하여 수정하려고합니다. 이것은 거꾸로입니다. 식별 할 때까지 이미 오류가 발생했습니다. 이를위한 코드 검토 / 컴파일러 강제 솔루션은 보호 조건이 아니라 getter 및 setter 또는 const 멤버입니다.


답변

다른 사람들이 지적했듯이, 이것은 합법적으로msgSender 할 수 있는지 여부에 달려 있습니다 . 다음은 가정해야한다고 가정합니다. NULL NULL 아니 합니다.

void PowerManager::SignalShutdown()
{
    if (!msgSender_)
    {
       throw SignalException("Shut down failed because message sender is not set.");
    }

    msgSender_->sendMsg("shutdown()");
}

팀의 다른 사람들이 제안한 “수정”은 Dead Programs Tell No Lies 원칙에 위배됩니다. 버그는 실제로 찾기가 어렵습니다. 이전 문제를 기반으로 동작을 자동으로 변경하는 방법은 첫 번째 버그를 찾기 어렵게 할뿐만 아니라 자체의 두 번째 버그도 추가합니다.

주니어는 null을 확인하지 않아 혼란을 겪었습니다. 이 코드 조각이 정의되지 않은 상태로 계속 실행하여 혼란을 겪는다면 (장치는 켜져 있지만 프로그램은 “감지”합니다)? 아마도 프로그램의 다른 부분은 장치가 꺼져있을 때만 안전한 것을 할 것입니다.

이러한 접근 방식 중 하나는 자동 실패를 방지합니다.

  1. 이 답변에서 제안한대로 어설 션을 사용하십시오. 에서 하지만 프로덕션 코드에서 설정되어 있는지 확인하십시오. 물론 다른 생산자들이 생산에서 제외 될 것이라는 가정하에 작성된 경우 문제가 발생할 수 있습니다.

  2. null 인 경우는 예외를 throw합니다.