Thursday, May 10, 2007

Case .. else raise

I believe in safe programming. In my book that means that my programs should fail whenever I do something wrong. Even better - they should automatically detect my future stupidities and warn me when I make them.

For example, take this completely made-up example:

type
TEnumeration = (enOne, enTwo, enThree);

function DoSomethingWith(enum: TEnumeration): integer;
begin
case enum of
enOne: Result := 1;
enTwo: Result := 2;
enThree: Result := 3;
end;
end;

Pretty typical, huh? We'll, I won't write it like that. Never.


Let's say that in the future TEnumeration gets extended to include enFour. Now you have to hunt all places where it is used and adapt the code. Maybe you'll miss one or two, maybe not. I usually do :(


At least in fragments like the one above, a little planning can save you hours of debugging. By adding just one line, you can be automatically reminded that you failed to update the code at this very place:

type
TEnumeration = (enOne, enTwo, enThree);

function DoSomethingWith(enum: TEnumeration): integer;
begin
case enum of
enOne: Result := 1;
enTwo: Result := 2;
enThree: Result := 3;
else raise Exception.Create('DoSomethingWith: Unexpected value');
end;
end;

(For commenters: Yes, I'm aware that this is simply a precondition check.)


[

Technorati tags: , ,
]

10 comments:

  1. Anonymous01:03

    Assert would be more appropriate here.

    It would allow you to catch the error at runtime in testing AND get actual line numbers and it will allow you to ensure that code you deploy never throws these kinds of errors in deployed code.

    ReplyDelete
  2. Why don't u just use Range Chek compiler option ? :)

    ReplyDelete
  3. Rodrigo05:56

    Why not ?

    type
    TEnumeration = (enOne, enTwo, enThree);

    function DoSomethingWith(enum: TEnumeration): integer;
    begin
    case enum of
    enOne..enThree:
    Result := Ord(enum)+1
    else raise Exception.Create('DoSomethingWith: Unexpected value');
    end;
    end;

    ReplyDelete
  4. Anonymous07:35

    Personally i try, whenever possible and meaningful, to write code like this

    type
    TEnumeration = (enOne, enTwo, enThree);

    function DoSomethingWith(enum: TEnumeration): integer;
    const
    ResValues: array[TEnumeration] of integer = (1, 2, 3);
    begin
    Result := ResValues[enum];
    end;

    Now the compiler will hit me if the TEnumeration is extended, and i know from long experience, that the compiler is a lot wiser than i.

    ReplyDelete
  5. I don't like asserts.

    If there is an error in the code, the code must die. It is of no use if the program continues to work and generates wrong result. That's why I'm coding almost all pre/postconditions as hard exceptions.

    I'm using asserts only occasionaly, for catching conditions that should not happen but which don't cause the program to malfunction.

    ReplyDelete
  6. @undefined: Of what use would that be? There is no range check error here.

    ReplyDelete
  7. @rodrigo: Because that was not a point of this exercise. I was talking about a general approach here. Usually such case statements appear when it is not possible to generalize the code in case statement.

    ReplyDelete
  8. Anonymous17:34

    Exception and assertion catch the error at run time only. This kind of problem can be caught at compile time.

    function TForm1.DoSomethingWith(enum: TEnumeration): integer;
    begin
    {$if (((Low(TEnumeration) <> enOne) or
    (High(TEnumeration) <> enThree)))}
    {$Message Error 'enum: TEnumeration range uncomplete management'}
    {$ifend}
    case enum of
    enOne: Result := 1;
    enTwo: Result := 2;
    enThree: Result := 3;
    end;
    end;

    ReplyDelete
  9. This is an excellent idea and I'll surely adopt it.

    The only problem is that it fails if TEnumeration is extended as

    TEnumeration = (enOne, enTwo, enFour, enThree);

    Yeah, I know, this _should_ not be done. Still ...

    ReplyDelete