Never say never!


  • Discourse touched me in a no-no place

    Had an error report come through today that caught my eye. I looked in the codebase to see where the error was coming from, to find this...

    if (!IsRegister2TempDataValid())
    {
      return RedirectToAction("Index");
    }
    
    var member = RegistrationService.GetMemberByCard(Register1Model.CardNumber);
    if (member == null)
    {
      throw new Exception(
        "Member not found - although the member was found previously so this should never happen!");
    }
    

    :rolleyes:


  • Winner of the 2016 Presidential Election

    @DoctorJones
    And that, kids, is why distinguishing nullable references from non-nullable ones in the type system is a good thing.



  • @asdf said in Never say never!:

    @DoctorJones
    And that, kids, is why distinguishing nullable references from non-nullable ones in the type system is a good thing.

    I don't see how that would actually affect this. If I understand the code correctly it's attempting to retrieve the member from a service of some kind. Which means that you would have to always have a nullable reference returned from that method, since you could be attempting to retrieve an object that isn't there. The only reason I assume the code says that it should never happen is that the author of the code either assumed that nothing would remove the member between calls, or was assuming some sort of transactional guarantee that is no longer true.


  • Winner of the 2016 Presidential Election

    @DescentJS
    In that case, the error message needs more "SPANK!".



  • @DescentJS said in Never say never!:

    @asdf said in Never say never!:

    @DoctorJones
    And that, kids, is why distinguishing nullable references from non-nullable ones in the type system is a good thing.

    I don't see how that would actually affect this. If I understand the code correctly it's attempting to retrieve the member from a service of some kind. Which means that you would have to always have a nullable reference returned from that method, since you could be attempting to retrieve an object that isn't there. The only reason I assume the code says that it should never happen is that the author of the code either assumed that nothing would remove the member between calls, or was assuming some sort of transactional guarantee that is no longer true.

    Which is why the error should be a custom exception type, with a description of what's actually wrong. Everything is wrong with the branch of logic that throws this exception.


Log in to reply