Access Violation beim hinzufügen einer neuen TDirecotory

[milos]

Hallo,
ich habe hier ein object, welches ich erstellt habe um unsere Geräte und dessen Zubehör aufzulisten.
Ich habe eine CSV Liste vorliegen die ich Parsen will und jedes Gerät in einer Liste voller TDevice's stecke.

Ich habe das Problem das wenn ich eine neue Directory in einer List adden will, kommt beim debuggen bei der markierter stelle im code ein Access Violation. Ich habe leider keine ahnung wieso dieser fehler entsteht, für mich sieht alles richtig aus :/

Das lustige ist, dass dieser Fehler erst beim 2. aufruf passiert.

Delphi-Quellcode:

unit Device;
 
interface
  uses
    Sysutils, System.Generics.Collections;
 
  type
  TDevices = class
    DeviceName : string;
  private
    { Private-Deklarationen }
  public
    AList: TList<TDictionary<string, string>>;
    procedure SetDeviceName(Device : string);
    procedure AddAccessories(Name : string; Description: string; Price: string; OType:string);
  end;
var
  Name: string;
 
implementation
 
{ Devices }
 
procedure TDevices.AddAccessories(Name, Description, Price, OType: string);
begin
  AList.Add(TDictionary<string, string>.Create()); // <------- Hier wird beim 2. Aufruf eine Access Violation gemeldet.
  AList[AList.Count-1].Add('name', Name);
  AList[AList.Count-1].Add('description', Description);
  AList[AList.Count-1].Add('price', Price);
  AList[AList.Count-1].Add('type', OType);
end;
 
procedure TDevices.SetDeviceName(Device: string);
begin
  Name:=Device;
  AList := TList<TDictionary<string, string>>.Create();
 
end;
 
end.

Ich hoffe ihr könnt mir helfen

MfG

[Union]

Du vergisst sicher SetDeviceName aufzurufen.

[milos]

Nein, das ist eben nicht das Problem, habe ich mir jedoch auch schon gedacht ^^
Womöglich habe ich einen Fehler bei der Programmierung im Parser :-/ Ich suche mal danach, wäre jedoch trotzdem für jeden Tipp dankbar

MfG

Edit: Wow, kaum kommt es mir in den Sinn, dass auch im Parser ein fehler sein könnte, finde ich ihn -.-"
Habe immer ein neues TDevices Objekt erstellt, und SetDeviceName() wird nur aufgerufen sobald ein neues Gerät kommt, dass er noch nicht kennt. Schönen Fehler hab ich mir gemacht, der mir so viel Zeit gekostet hat -.-"

Naja, danke trotzdem und sry für meine dummheit xD

[himitsu]

Und jetzt erstellst du AList schon im Constructor?



SetDeviceName setzt "Name", aber nicht "DeviceName".
Ist das so gewollt?

Seh ich das richtig, oder gibt es da wirklich eine globale Variable mit dem Namen "Name"?

Wer gibt die erzeugten TDictionary-Instanzen eigentlich wieder frei?
Da wäre es bestimmt besser, wenn du eine TObjectList nimmst, statt der TList. Und Dieser sagst, daß sie die freigeben soll. (OwnsObjects=True)

Ach ja, man glaubt garnicht, was man an "mehrfachen" Aufrufen einsparen kann.

Delphi-Quellcode:

procedure TDevices.AddAccessories(Name, Description, Price, OType: string);
var
  i: Integer;
begin
  i := AList.Add(TDictionary<string, string>.Create);
  AList[i].Add('name', Name);
  AList[i].Add('description', Description);
  AList[i].Add('price', Price);
  AList[i].Add('type', OType);
end;

procedure TDevices.AddAccessories(Name, Description, Price, OType: string);
var
  i: Integer;
begin
  //i := AList.Add(TDictionary<string, string>.Create);
  //AList[i].Items['name'] := Name;
  //AList[i].Items['description'] := Description;
  //AList[i].Items['price'] := Price;
  //AList[i].Items['type'] := OType;

  i := AList.Add(TDictionary<string, string>.Create);
  AList[i]['name'] := Name;
  AList[i]['description'] := Description;
  AList[i]['price'] := Price;
  AList[i]['type'] := OType;
end;

procedure TDevices.AddAccessories(AName, ADescription, APrice, AType: string);
var
  i: Integer;
begin
  with AList[AList.Add(TDictionary<string, string>.Create)] do
  begin
    Items['name'] := AName;
    Items['description'] := ADescription;
    Items['price'] := APrice;
    Items['type'] := AType;
  end;
end;

PS: TObjectDictionary<string,xxx> statt TObjectList<xxx>, wobei der String dann den DeviceName enthält (zusätzlich zum DeviceName in xxx), dann kannst du auch noch die Devices recht schnell in der Liste suchen lassen.

[milos]

Hi,

Nein, AList wird immer noch bei SetDeviceName() erstellt und der Name war gewollt, aber blöd Formuliert ^^ Danke, dachte nicht dran ^^

Hmm, bin noch nicht sehr gut in Delphi und wusste nicht das ich Globale Variablen definiert habe.
Dies geschieht doch indem ich sie in einen var Block im Interface setze oder? Sollte ich die besser bei Implementation definieren? Oder sogar bei dem private block?

Danke mit dem Tipp mit dem sparen von Aufrufen und dem TObjectList, werde ich mir merken. ^^

[jaenicke]

Zitat von milos:

Dies geschieht doch indem ich sie in einen var Block im Interface setze oder? Sollte ich die besser bei Implementation definieren? Oder sogar bei dem private block?

Da du sie außerhalb der Klasse deklariert hast, gehört diese nicht zur Klasse, sondern ist global. Sprich, wenn du dann mal zwei solche Objekte erstellst, hast du diese eine Variable für beide.
Dann steht da immer das drin, was zuletzt zugewiesen wurde.

Noch dazu ist der Name für die Variable äußerst ungünstig gewählt. Denn die Eigenschaft Name hat jede Komponente, auch ein Formular. Wenn du jetzt Code aus deiner separaten Unit z.B. in dein Formular kopierst, kompiliert er zwar, aber du nutzt, ggf. ohne es zu merken, die Eigenschaft Name des Formulars.

Bei DeviceName hast du keinen Sichtbarkeitsmodifizierer angegeben, so dass published benutzt wird. Denn so kann man direkt an diese Variable bzw. dieses Feld heran.

Besser ist es so:

Delphi-Quellcode:

type
  TDevices = class
  private
    FDeviceName: string; // ein großes F steht lauf Konvention immer vor privaten Feldern
    FList: TObjectList<TDictionary<string, string>>; // Die Liste selbst wird von TDevices verwaltet, ist deshalb nur via Eigenschaft erreichbar
    procedure SetDeviceName(const ADevice: string);
  public
    constructor Create;
    destructor Destroy; override;
    procedure AddAccessories(const ADeviceName, ADescription, APrice, AType: string); // ein großes A kennzeichnet Parameter
    property DeviceName: string read FDeviceName write SetDeviceName;
    property List: TObjectList<TDictionary<string, string>> read FList;
  end;

constructor TDevices.Create;
begin
  FList := TObjectList<TDictionary<string, string>>.Create(True);
end;

destructor TDevices.Destroy;
begin
  FList.Free;
  inherited;
end;