views:

133

answers:

2

I have a status which is stored as a string of a set length, either in a file or a database.

I'm looking to enumerate the possible status'

I have the following type to define the possible status'

Type TStatus = (fsNormal = Ord('N'),fsEditedOnScreen = Ord('O'),
                fsMissing = Ord('M'),fsEstimated = Ord('E'),fsSuspect = Ord('s'),
                fsSuspectFromOnScreen = Ord('o'),fsSuspectMissing = Ord('m'),
                fsSuspectEstimated = Ord('e'));

Firstly is this really a good idea? or should I have a seperate const array storing the char conversions? That would mean more than one place to update.

Now convert a string to a status array I have the following, but how can I check if a char is valid without looping through the enumeration?

Function StrToStatus(Value : String):TStatusArray;
var
    i: Integer;
begin
    if Trim(Value) = '' then
    begin
        SetLength(Result,0);
        Exit;
    end;
    SetLength(Result,Length(Value));
    for i := 1 to Length(Value) do
    begin
        Result[i] := TStatus(Value[i]); // I don't think this line is safe.
    end;
end;

After some testing it sames the suspect line is safe (it doesn't crash!) but just adds in (out of bounds) values which then need filtering out.

Function StrToStatus(Value : String):TStatusArray;
var
    i: Integer;
begin
    if Trim(Value) = '' then
    begin
        SetLength(Result,0);
        Exit;
    end;
    SetLength(Result,Length(Value));
    for i := 1 to Length(Value) do
    begin
        Result[i-1] := TStatus(Value[i]);
    end;
    for i := 0 to Length(Result) - 1 do
    begin
        case Result[i] of
            fsNormal: ;
            fsEditedOnScreen: ;
            fsMissing: ;
            fsEstimated: ;
            fsSuspect: ;
            fsSuspectFromOnScreen: ;
            fsSuspectMissing: ;
            fsSuspectEstimated: ;
            else
                Result [i] := fsNormal;
        end;
    end;
end;

This allows all the status' and their relative Char values to be in one place and prevents looping through every status for every character in the string. (So in my head atleast should be a bit faster)

AFAIK this should be fine for converting back again.

Function StatusToStr(Value : TStatusArray):String;
var
  i: Integer;
begin
    for i := 0 to Length(Value) - 1 do
        Result := Result + Chr(Ord(Value[i]))
end;

I'm using Delphi 2007

A: 

First, I wonder why you save it as string instead of as integer.

The way you've done it, the only way to do it correctly would be to have a Case condition...

function CharToStatus(AChar : Char):TStatus;
begin
  case AChar of
    'N' : Result := fsNormal;
    'O' : Result := fsEditedOnScreen;
    'M' : Result := fsMissing;
    'E' : Result := fsEstimated;
    's' : Result := fsSuspect;
    'o' : Result := fsSuspectFromOnScreen;
    'm' : Result := fsSuspectMissing;
    'e' : Result := fsSuspectEstimated;
  else
    //Manage error;
  end;
end;

function StatusToChar(AStatus : TStatus) : char;
begin
  Result := Char(AStatus);
end;

The expression x in [Low(TStatus)]..High(Tstatus)] won't work in this situation. The reason for this is that Low(TStatus) = 'E', and High(TStatus) = 's'. Anything in-between would be considered valid. (i.e. 'Z' is in [Low(TStatus)]..High(Tstatus)])

The expression x in [Low(TStatus)]..High(Tstatus)] only work on type where there is no "hole" in the declaration. (Like those without explicit values, where the first element is 0, 2nd is 1, 3rd is 2... etc)

//EDIT

Ok.. thinking the problem a bit further, I don't see why you don't like the const array approach... Something like this would be a lot better.

type
  TStatus = (fsNormal, fsEditedOnScreen,
                fsMissing,fsEstimated,fsSuspect,
                fsSuspectFromOnScreen,fsSuspectMissing ,
                fsSuspectEstimated);
const
  StatusValue : Array[TStatus] of Char = ('N','O','M','E','s','o','m','e');

function StatusValueToTStatus(C : Char) : TStatus;
var I : Integer;
begin
  for I := Low(StatusValue) to High(StatusValue)  do
  begin
    if StatusValue = C then
    begin
      Result := TStatus(I);
      EXIT;
    end;
  end;
  //Not found, Manage errors
end;
Ken Bourassa
It's saved as a string as part of a legacy system. The reason I didn't go for the const approach was simply that I can see other developers breaking it by being a little careless when they add new status'
JamesB
If they don't adjust the array length after adding a new enum value the code won't compile.
Ulrich Gerhardt
+2  A: 

If I understand you correctly I would replace the array with a set and use an enum without explicit values, like so:

program Project1;

{$APPTYPE CONSOLE}

uses
  SysUtils;

type
  TStatus = (fsNormal, fsEditedOnScreen, fsMissing, fsEstimated, fsSuspect,
    fsSuspectFromOnScreen, fsSuspectMissing, fsSuspectEstimated);
  TStatusSet = set of TStatus;

const
  cStatusChars: array[TStatus] of Char = ('N', 'O', 'M', 'E', 's', 'o', 'm', 'e');

function CharToStatus(AChar: Char; out AStatus: TStatus): Boolean;
var
  st: TStatus;
begin
  for st := Low(TStatus) to High(TStatus) do
    if cStatusChars[st] = AChar then
    begin
      AStatus := st;
      Result := True;
      Exit;
    end;
  Result := False;
end;

function StrToStatus(const Value: string): TStatusSet;
var
  i: Integer;
  st: TStatus;
begin
  Result := [];
  for i := 1 to Length(Value) do
    if CharToStatus(Value[i], st) then
      Include(Result, st);
end;

function StatusToStr(const Value: TStatusSet): string;
var
  st: TStatus;
begin
  for st in Value do
    Result := Result + cStatusChars[st];
end;

var
  StatusSet: TStatusSet;
begin
  StatusSet := StrToStatus('EmO');
  Writeln(StatusToStr(StatusSet));
  Readln;
end.
Ulrich Gerhardt
I need the result to be an array instead of set (The string represents the different status of multiple values, each value can only have one status). Apart from that should I have performance concerns with the extra loop?
JamesB
By extra loop you mean the one in CharToStatus? You can replace it by constructing a lookup array `array[Char] of TStatus` before you use StrToStatus the first time. I have no idea if this would speed things up.
Ulrich Gerhardt