Why does TOmniValue not accept named values with length of name equal to 1?

Question

The following program results in an error, raised in the execution of TOmniValue.CreateNamed.

{$APPTYPE CONSOLE}

uses
  OtlCommon;

var
  Value: TOmniValue;

begin
  Value := TOmniValue.CreateNamed([
    'a', 42,
    'b', 666
  ]);
  Writeln(Value['a'].AsString);
  Writeln(Value['b'].AsString);
end.

The exception is of type Exception with message:

TOmniValue.CreateNamed: invalid name type

If the names are more than a single character in length then the code runs without error and reports the expected output.

Is there a problem with my code, or is there a problem with the library?

Do we build some central bug reporting system here ? If so, I have a few reports of my favorite FOSS product (that I've reported to the author). Just need to know if should I warn about corner cases that I found on SO... — TLama, Jun 03 '14 at 01:09
Well, I would call it corner case since using 1 char for name looks pointless to me (and indicates a code smell in my view). But it's great that you found and precisely described this issue and I definitely disagree with the downvote here. — TLama, Jun 03 '14 at 07:55
@TLama It came up when investigating this Q (http://stackoverflow.com/questions/23994202/error-when-using-strings-in-tomnivalue-array-in-a-backgroundworker) Note the key names used there. Something tells me I was not the first to spot the issue. — David Heffernan, Jun 03 '14 at 13:51

David Heffernan · Accepted Answer · 2014-06-02T15:06:15.020

This appears to be a problem with the library. The implementation of CreateNamed is:

constructor TOmniValue.CreateNamed(const values: array of const;
  const cppDupConWorkaround: boolean);
var
  i   : integer;
  name: string;
  ovc : TOmniValueContainer;
begin
  ovc := TOmniValueContainer.Create;
  Assert(not Odd(Low(values)));
  name := '';
  for i := Low(values) to High(values) do begin
    with values[i] do begin
      if not Odd(i) then
        case VType of
          vtChar:          name := string(VChar);
          vtString:        name := string(VString^);
          vtPChar:         name := string(StrPasA(VPChar));
          vtAnsiString:    name := string(VAnsiString);
          vtVariant:       name := string(VVariant^);
          vtWideString:    name := WideString(VWideString);
          {$IFDEF UNICODE}
          vtUnicodeString: name := string(VUnicodeString);
          {$ENDIF UNICODE}
        else
          raise Exception.Create ('TOmniValue.CreateNamed: invalid name type')
        end //case
      else
        case VType of
          vtInteger:       ovc.Add(VInteger, name);
          vtBoolean:       ovc.Add(VBoolean, name);
          vtChar:          ovc.Add(string(VChar), name);
          vtExtended:      ovc.Add(VExtended^, name);
          vtString:        ovc.Add(string(VString^), name);
          vtPointer:       ovc.Add(VPointer, name);
          vtPChar:         ovc.Add(string(StrPasA(VPChar)), name);
          vtAnsiString:    ovc.Add(AnsiString(VAnsiString), name);
          vtCurrency:      ovc.Add(VCurrency^, name);
          vtVariant:       ovc.Add(VVariant^, name);
          vtObject:        ovc.Add(VObject, name);
          vtInterface:     ovc.Add(IInterface(VInterface), name);
          vtWideString:    ovc.Add(WideString(VWideString), name);
          vtInt64:         ovc.Add(VInt64^, name);
          {$IFDEF UNICODE}
          vtUnicodeString: ovc.Add(string(VUnicodeString), name);
          {$ENDIF UNICODE}
        else
          raise Exception.Create ('TOmniValue.CreateNamed: invalid data type')
        end; //case
    end; //with
  end; //for i
  SetAsArray(ovc);
end; { TOmniValue.CreateNamed }

The exception is raised by the first of the two raise statements above. The exception is used to indicate that a value was supplied with a type that cannot be handled. As it turns out, the type of the value when you specify a string literal with length 1 is vtWideChar. And indeed this type is not handled at all.

So you can work around the issue by forcing the call to CreateNamed to receive strings rather than single characters:

Value := TOmniValue.CreateNamed([
  string('a'), 42,
  string('b'), 666
]);

In my view it would be better for the library to be modified like to accept single characters. It already handles AnsiChar and I suspect it is a simple omission that it does not handle WideChar. I think the code should read:

constructor TOmniValue.CreateNamed(const values: array of const;
  const cppDupConWorkaround: boolean);
var
  i   : integer;
  name: string;
  ovc : TOmniValueContainer;
begin
  ovc := TOmniValueContainer.Create;
  Assert(not Odd(Low(values)));
  name := '';
  for i := Low(values) to High(values) do begin
    with values[i] do begin
      if not Odd(i) then
        case VType of
          vtChar:          name := string(VChar);
          vtString:        name := string(VString^);
          vtPChar:         name := string(StrPasA(VPChar));
          vtAnsiString:    name := string(VAnsiString);
          vtVariant:       name := string(VVariant^);
          vtWideString:    name := WideString(VWideString);
          vtWideChar:      name := string(VWideChar);
          {$IFDEF UNICODE}
          vtUnicodeString: name := string(VUnicodeString);
          {$ENDIF UNICODE}
        else
          raise Exception.Create ('TOmniValue.CreateNamed: invalid name type')
        end //case
      else
        case VType of
          vtInteger:       ovc.Add(VInteger, name);
          vtBoolean:       ovc.Add(VBoolean, name);
          vtChar:          ovc.Add(string(VChar), name);
          vtExtended:      ovc.Add(VExtended^, name);
          vtString:        ovc.Add(string(VString^), name);
          vtPointer:       ovc.Add(VPointer, name);
          vtPChar:         ovc.Add(string(StrPasA(VPChar)), name);
          vtAnsiString:    ovc.Add(AnsiString(VAnsiString), name);
          vtCurrency:      ovc.Add(VCurrency^, name);
          vtVariant:       ovc.Add(VVariant^, name);
          vtObject:        ovc.Add(VObject, name);
          vtInterface:     ovc.Add(IInterface(VInterface), name);
          vtWideString:    ovc.Add(WideString(VWideString), name);
          vtWideChar:      ovc.Add(string(VWideChar), name);
          vtInt64:         ovc.Add(VInt64^, name);
          {$IFDEF UNICODE}
          vtUnicodeString: ovc.Add(string(VUnicodeString), name);
          {$ENDIF UNICODE}
        else
          raise Exception.Create ('TOmniValue.CreateNamed: invalid data type')
        end; //case
    end; //with
  end; //for i
  SetAsArray(ovc);
end; { TOmniValue.CreateNamed }

Reported as: OTL issue #64

No need to put the new code inside the ifdef; Delphi has accepted WideChar values since version 3. Might want to handle PWideChar while you're at it. — Rob Kennedy, Jun 02 '14 at 14:40
@RobKennedy In the pre-Unicode case you'd have to use `WideString`, I guess. — David Heffernan, Jun 02 '14 at 14:51
Not really. If the existing WideString-to-AnsiString assignment is acceptable in the vtWideString case, then a WideChar-to-AnsiString assignment should be acceptable in the vtWideChar case. — Rob Kennedy, Jun 02 '14 at 14:59

Why does TOmniValue not accept named values with length of name equal to 1?

1 Answers1