How can I avoid EInvalidPointer error when using TObjectDictionary in Delphi?

Question

The program receives product information datas through window message. Incoming datas processed in TProductInstance.PutProductData procedure.

Product information contains date, name, price. I want to store datas as TObjectDictionary. The key is same date of the product and value is product information data list as TObjectList. Also I want to maintain datas only in latest 7 days. By the way, when I remove the item from TObjectDictionary for maintaining, error occurs like below.

First chance exception at $75214598.
Exception class EInvalidPointer with message 'Invalid pointer operation'. Process product.exe (3848).

This is caused by FProductDictionary.Remove(StringKey);.

How can I avoid EInvalidPointer error with maintain latest 7 days datas?

type
  TProductItem = class(TObject)
  private
    FDate: String;
    FName: String;
    FPrice: Integer;
    procedure SetDate(const value: String);
    procedure SetName(const value: String);
    procedure SetPrice(const value: Integer);
  public
    property Date: String read FDate write SetDate;
    property Name: String read FName write SetName;
    property Price: Integer read FPrice write SetPrice;
    constructor Create(const date, name: String; const price: Integer);
  end;

  TProductItemList = class(TObjectList<TProductItem>);

type
  TProductInstance = class(TObject)
  private
  public
    FLatestDate: String;

    FProductList: TProductItemList;
    FProductDictionary: TObjectDictionary<String, TProductItemList>;

    constructor Create;
    destructor Destroy; override;

    procedure PutProductData(var Data: LP_Data);
  end;

implementation

constructor TProductInstance.Create;
begin
  FLatestDate := '';

  FProductList := TProductItemList.Create;
  FProductDictionary := TObjectDictionary<String, TProductItemList>.Create([doOwnsValues]);
end;

procedure TProductInstance.PutProductData(var Data: LP_Data);
var
  StringKey: String;
begin
  if (Trim(LP_Data^.date) <> FLatestDate) then
  begin
    FProductDictionary.AddOrSetValue(Trim(LP_Data^.date), FProductList);
    for StringKey in FProductDictionary.Keys do
    begin
      if (GetDateToInt(Trim(LP_Data^.date)) - GetDateToInt(FLatestDate) > 7) then
        FProductDictionary.Remove(StringKey);
    end;
    FProductList.Free;
  end;
  FProductList.Add(TProductItem.Create(Trim(LP_Data^.date), Trim(LP_Data^.name), Trim(LP_Data^.price)));
  FLatestDate := Trim(LP_Data^.date);
end;

UPDATED

type
  TProductItem = class(TObject)
  private
    FDate: String;
    FName: String;
    FPrice: Integer;
    procedure SetDate(const value: String);
    procedure SetName(const value: String);
    procedure SetPrice(const value: Integer);
  public
    property Date: String read FDate write SetDate;
    property Name: String read FName write SetName;
    property Price: Integer read FPrice write SetPrice;
    constructor Create(const date, name: String; const price: Integer);
  end;


type
  TProductInstance = class(TObject)
  private
  public
    FLatestDate: String;

    FProductList: TObjectList<TProductItem>;
    FProductDictionary: TObjectDictionary<String, TObjectList<TProductItem>>;

    constructor Create;
    destructor Destroy; override;

    procedure PutProductData(var Data: LP_Data);
  end;

implementation

constructor TProductInstance.Create;
var
  LProductItem: TProductItem;
  LProductItemList: TObjectList<TProductItem>;
  LStringList: TStringList;
begin
  FLatestDate := '';

  FProductList := TObjectList<TProductItem>.Create;
  FProductDictionary := TObjectDictionary<String, TObjectList<TProductItem>>.Create([doOwnsValues]);
end;

procedure TProductInstance.PutProductData(var Data: LP_Data);
var
  StringKey: String;
begin
  FProductList.Add(TProductItem.Create(Trim(LP_Data^.date), Trim(LP_Data^.name), Trim(LP_Data^.price)));  
  if (Trim(LP_Data^.date) <> FLatestDate) then
  begin
    LProductItemList := TObjectList<ProductItem>.Create;
    for LProductItem in FProductList do
    begin
      LProductItemList.Add(LProductItem);
    end;

    FProductDictionary.AddOrSetValue(Trim(LP_Data^.date), LProductItemList);
    FProductList.Clear;

    LStringList := TStringList.Create;
    for StringKey in FProductDictionary.Keys do
    begin
      if (GetDateToInt(Trim(LP_Data^.date)) - GetDateToInt(FLatestDate) > 7) then
      begin
        LStringList.Add(StringKey);
      end;
    end;
    for StringKey in LStringList do
    begin
      FProductDictionary.Remove(StringKey);
    end;

    FreeAndNil(LStringList);
  end;
end;

Updated code occurs EInvalidPointer error on FProductDictionary.Remove(StringKey); What did I wrong?

Answer 1

The code you present is incomplete. You did not show the destructor for TProductInstance. For a question such as this you should always supply a simple MCVE. This is quite easy to achieve in a single console .dpr file.

Looking at what we can see, it is clear that the lifetime management in the code is broken. Let us critique this method.

procedure TProductInstance.PutProductData(var Data: LP_Data);
var
  StringKey: String;
begin
  if (Trim(LP_Data^.date) <> FLatestDate) then
  begin
    FProductDictionary.AddOrSetValue(Trim(LP_Data^.date), FProductList);
    for StringKey in FProductDictionary.Keys do
    begin
      if (GetDateToInt(Trim(LP_Data^.date)) - GetDateToInt(FLatestDate) > 7) then
        FProductDictionary.Remove(StringKey);
    end;
    FProductList.Free;
  end;
  FProductList.Add(TProductItem.Create(Trim(LP_Data^.date), Trim(LP_Data^.name), 
    Trim(LP_Data^.price)));
  FLatestDate := Trim(LP_Data^.date);
end;

Because FProductDictionary owns its values, when you do

FProductDictionary.AddOrSetValue(Trim(LP_Data^.date), FProductList);

then FProductDictionary becomes the owner of FProductList. That means that you should not destroy FProductList ever. However, you do exactly that:

FProductList.Free;

So you are going to be destroying FProductList multiple times which is a clear error.

What to do next? You need to deal with the lifetime issues. I cannot know from the code presented here what you are trying to achieve, and how the lifetime should be managed. You will need to work out who is responsible for owning what, and make sure that you stick to a clear lifetime management policy.

On the face of it, my best guess would be that you need to remove the FProductList field. When you need to add a new item to FProductDictionary, instantiate a new instance of TProductItemList, populate it, and add it to the dictionary. At that point the dictionary takes control of the lifetime of the TProductItemList.

As one final comment, I would suggest that the type TProductItemList is pointless. I would remove it. Use TObjectList<TProductItem> to make the code clearer to the reader. The reader can look at TObjectList<TProductItem> and know immediately what it is, since TObjectList<T> is such a ubiquitous type.