fabriciocolombo / delphi-rest-client-api

A Delphi REST client API to consume REST services written in any programming language.
Apache License 2.0
380 stars 182 forks source link

Method TOldRttiMarshal.ToClass do not populates object's properties, just public fields. #93

Closed fabiolindemberg closed 7 years ago

fabiolindemberg commented 7 years ago

Hi Fabrício, I downloaded the last version and installed it in Delphi 2010. I notice that properties of my objects weren't populated, so I had to changed some piece of code to it works. You can see changes below. In my undertanding about object oriented programming, classes didn't should expose fields directly. So, If you agree with if you can incorporate my code in your component.

function TOldRttiMarshal.ToClass(AObject: TObject): ISuperObject;
var
  i: Integer;
  vTypeInfo: PTypeInfo;
  vTypeData: PTypeData;
  vPropList: PPropList;
  vPropInfo: PPropInfo;
  value: ISuperObject;
  vObjProp: TObject;
  vAdapter: IJsonListAdapter;
begin
  if AObject = nil then
  begin
    Result := SuperObject.SO();
    Exit;
  end;

  if (AObject is TList) then
  begin
    Result := ToList(TList(AObject));
  end
  else if Supports(AObject, IJsonListAdapter, vAdapter) then
  begin
    Result := ToList(TList(vAdapter.UnWrapList));
  end
  else
  begin
    vTypeInfo := AObject.ClassInfo;

    if vTypeInfo = nil then
    begin
      raise ENoSerializableClass.Create(AObject.ClassType);
    end;

    vTypeData := GetTypeData(vTypeInfo);

    Result := SO();

    New(vPropList);
    try
      GetPropList(vTypeInfo, tkProperties, vPropList);

      for i := 0 to vTypeData^.PropCount-1 do
      begin
        vPropInfo := vPropList^[i];

        value := nil;

        case vPropInfo^.PropType^.Kind of
          tkMethod: ;
          tkSet, tkInteger, tkEnumeration: value := ToInteger(AObject, vPropInfo);
          tkInt64: value := ToInt64(AObject, vPropInfo);
          tkFloat: value := ToFloat(AObject, vPropInfo);
          tkChar, tkWChar: value := ToChar(AObject, vPropInfo); 
          tkString, tkLString,
          {$IFDEF UNICODE}
          tkUString,
          {$ENDIF}
          tkWString: value := ToJsonString(AObject, vPropInfo);
          tkClass: begin
                      value := nil;
                      vObjProp := GetObjectProp(AObject, vPropInfo);
                      if Assigned(vObjProp) then
                      begin
                        if vObjProp is TList then
                          value := ToList(TList(vObjProp))
                        else
                          value := ToClass(vObjProp);
                      end;
                   end;
        end;

        if Assigned(value) then
        begin
          Result.O[String(vPropInfo^.Name)] := value;
        end;
      end;
    finally
      Dispose(vPropList);
    end;
  end;
end;
fabriciocolombo commented 7 years ago

Hi @fabiolindemberg

I got your point, but this is by design. The first marshaller was SuperObject, who uses fields for serialization. Thats why TOldRttiMarshal also use it. Besides that, Delphi TJSONMarshal are also based on object fields.

We can't just change this to use properties instead, because we need to maintain compatibility. If you want to give a try, you should change all supported Marshalls, and make this option configurable, making serialization using fields the default option.

Anyway, these objects in general are not domain objects, they are Data Transfer Objects(DTO), created with purpose for transferring data over the wire. So, for me there is no problem exposing the fields.

IMHO, i think the current solution is good enought, and the effort to change (and mantain) the Marshalls not worth it.

Feel free to send a pull request.