The little things…

Honestly, for how many versions now has the following got through?

unit FMX.Types;

//...

type
  TGradientPoint = class(TCollectionItem)
  private
    FColor: TAlphaColor;
    FOffset: Single;
    function GetColor: TAlphaColor;
    procedure SetColor(const Value: TAlphaColor);
  public
    procedure Assign(Source: TPersistent); override;
    property IntColor: TAlphaColor read FColor write FColor;
  published
    property Color: TAlphaColor read GetColor write SetColor;
    property Offset: Single read FOffset write FOffset nodefault;
  end;

//...

procedure TGradientPoint.Assign(Source: TPersistent);
begin
  if Source is TGradientPoint then
  begin
    FColor := TGradientPoint(Source).FColor;
    FOffset := TGradientPoint(Source).FOffset;
  end
  else
    inherited;
end;

function TGradientPoint.GetColor: TAlphaColor;
begin
  Result := FColor;
end;

procedure TGradientPoint.SetColor(const Value: TAlphaColor);
begin
  FColor := Value;
end;

What am I whinging about you say? This:

  1. What’s with the weird IntColor/Color duplication? Probably an historical thing… but why wasn’t the IntColor version taken out when the Color version was refactored?
  2. Why does Color have a getter that just directly returns the value of the backing field?
  3. Why doesn’t its setter (and Assign) call the Changed method?
  4. Where’s the Add method for TGradientPoints to cast the TCollection implementation’s result to TGradientPoint?
  5. Where’s the Update override for TGradientPoints? We want a property change to make a visible difference, right?

Oh, and don’t get me started on how the TGradient property editor is both horrible to use and set up to replace (not complement) what would have been perfectly reasonable default Object Inspector behaviour…

Advertisements

8 thoughts on “The little things…

  1. 1. Yes, I think it’s about the compatibility. In my opinion, FColor and TAlphaColor is more efficient than FOffset and Single. It’s clear to read.
    2. I think this is a reserve. Embarcadero might put more code into this function in the future, so, they try to make it as functions.
    3 + 4 + 5. Don’t know why. haha

  2. 2: code written by people not knowing Pascal very well, or leftover from a refactoring.
    For 3 not to have performance implications, FMX would need a much better change, notification and invalidation propagation scheme than it currently has (the issue isn’t limited to this class).

    • “For 3 not to have performance implications”

      That’s overcomplicating things IMO. TCollection provides BeginUpdate and EndUpdate methods, which is enough here, given the case fits the purpose TCollection was designed for (configuring a collection of items at designtime and, occasionally, at runtime too).

      Of course, on the general point I agree. E.g., filling a FMX form with speed buttons will crash the application on a netbook when the VCL equivalent will load in a few seconds; also, see the TPathData implementation and weep…

  3. I think that IntColor is implemented as a way to be able to change color property from some ouside class without fireing OnChange event which as you noticed is not even implemented for some reason.

  4. Pingback: A FMX bug so stupid it must be sabotage | Words from a Delphi CodeSmith

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s