Mój program dla pisania CYOA

Cześć wszystkim! Jestem z Ukrainy ipiszę na Delphi.
Teraz robię program dla pisania tzw CYOA (Choose Your Own Adventure) książek. Ponieważ nie mam dużo doświadczenia w OOP, posądzę, że mój kod jest nie lepiej od spaghetti. Czy wolno, że będę pokazywał swój kod, a Państwo da mi wskazówki do polepszenia tego kodu?

Takie są moje klasy głowne:

TParagraph = class
  private
    FName: string;
    FParagraphText: TStringList;
    FLinks: TParagraphLinkList;
    FNotes: TStringList;
    FID: Integer;
    FNumber: Integer;
    FParentList: TParagraphList;
    function GetName: string;
    procedure SetName(const Value: string);
    function GetParagraphText: TStringList;
    procedure SetParagraphText(const Value: TStringList);
    function GetLinks: TParagraphLinkList;
    procedure SetLinks(const Value: TParagraphLinkList);
    function GetNotes: TStringList;
    procedure SetNotes(const Value: TStringList);
    function GetID: Integer;
    procedure SetID(const Value: Integer);
    function GetNumber: Integer;
    procedure SetNumber(const Value: Integer);
    function GetParentList: TParagraphList;
    procedure SetParentList(const Value: TParagraphList);
  public
    property name: string read GetName write SetName;
    property ParagraphText: TStringList read GetParagraphText write
      SetParagraphText;
    property Links: TParagraphLinkList read GetLinks write SetLinks;
    property Notes: TStringList read GetNotes write SetNotes;
    property ID: Integer read GetID write SetID;
    property Number: Integer read GetNumber write SetNumber;
    property ParentList: TParagraphList read GetParentList write SetParentList;
    //
    constructor Create;
    destructor Destroy; override;
    //
    function NumberOfWords: Integer;
    function IsEmpty: Boolean;
    function IsOrphan: Boolean;
    function IncomingLinks: TParagraphLinkList;
    function GetStatus: Integer;
    procedure ReplaceLinks;
  end;

  TParagraphListEnumerator = class
  private
    FList: TParagraphList;
    FCurrent: TParagraph;
    CurIndex: Integer;
  public
    constructor Create(AList: TParagraphList);
    function MoveNext: Boolean;
    property Current: TParagraph read FCurrent;
  end;

  TParagraphList = class
  private
    FList: TObjectList;
    FFirst: TParagraph;
    FGlobalNotes: TStringList;
    function GetItems(index: Integer): TParagraph;
    procedure SetItems(index: Integer; const Value: TParagraph);
    procedure SetFirst(const Value: TParagraph);
    function DoesThisNameExist(NameToCheck: string): Boolean;
    procedure DisambiguateName(Par: TParagraph);
  public
    property GlobalNotes: TStringList read FGlobalNotes write FGlobalNotes;
    property Items[index: Integer]: TParagraph read GetItems write SetItems;
    default;
    property First: TParagraph read FFirst write SetFirst;
    function Count: Integer;
    procedure Add(Addee: TParagraph); overload;
    function Add: TParagraph; overload;
    function IndexOf(Param: TParagraph): Integer;
    procedure Remove(Trash: TParagraph);
    //
    procedure Shuffle;
    //
    function EmptyParagraphs: TParagraphList;
    function EmptyCount: Integer;
    function NonEmptyParagraphs: TParagraphList;
    function NonEmptyCount: Integer;
    function NoChoicesParagraphs: TParagraphList;
    function NoChoicesCount: Integer;
    function OneChoiceParagraphs: TParagraphList;
    function OneChoicesCount: Integer;
    function TwoChoicesParagraphs: TParagraphList;
    function TwoChoicesCount: Integer;
    function ThreeChoicesParagraphs: TParagraphList;
    function ThreeChoicesCount: Integer;
    function ManyChoicesParagraphs: TParagraphList;
    function ManyChoicesCount: Integer;
    function LinksCount: Integer;
    function EmptyLinksCount: Integer;
    function GetEnumerator: TParagraphListEnumerator;
    function AllLinks: TParagraphLinkList;
    constructor Create;
    constructor CreateHolder;
    destructor Destroy; override;
  end;

  TParagraphLink = class
  private
    FSender: TParagraph;
    FLinkText: string;
    FTarget: TParagraph;
    function GetLinkText: string;
    function GetSender: TParagraph;
    function GetTarget: TParagraph;
    procedure SetLinkText(const Value: string);
    procedure SetSender(const Value: TParagraph);
    procedure SetTarget(const Value: TParagraph);
  public
    property Sender: TParagraph read GetSender write SetSender;
    property LinkText: string read GetLinkText write SetLinkText;
    property Target: TParagraph read GetTarget write SetTarget;
    constructor Create(Send: TParagraph; LText: string; Targ: TParagraph);
  end;

  TParagraphLinkListEnumerator = class
  private
    FList: TParagraphLinkList;
    FCurrent: TParagraphLink;
    CurIndex: Integer;
  public
    constructor Create(AList: TParagraphLinkList);
    function MoveNext: Boolean;
    property Current: TParagraphLink read FCurrent;
  end;

  TParagraphLinkList = class
  private
    FParentParagraph: TParagraph;
    FList: TObjectList;
    function GetItems(index: Integer): TParagraphLink;
    procedure SetItems(index: Integer; const Value: TParagraphLink);
  public
    property ParentParagraph
      : TParagraph read FParentParagraph write FParentParagraph;
    property Items[index: Integer]
      : TParagraphLink read GetItems write SetItems; default;
    function GetEnumerator: TParagraphLinkListEnumerator;
    procedure Add(Addee: TParagraphLink);
    procedure Delete(TrashIndex: Integer);
    procedure Remove(Trash: TParagraphLink);
    procedure Exchange(Ind1, Ind2: Integer);
    procedure MoveLinkUp(Ind: Integer);
    procedure MoveLinkDown(Ind: Integer);
    function Count: Integer;
    function CanAddLink: Boolean;
    constructor Create;
    constructor CreateHolder;
    destructor Destroy; override;
  end;

P. S. Ciała metod w załącznikach.

A ciała tych metod?

@shinkarom: if it better for your comfort, you can wrote in English here. Most people should understand it and answer you in that language. Because using Google Translate to direct translation English <-> Polish may return funny and strange results. I hope my bad English is enought in this case :)

Ви завжди можете написати українською мовою :]

هذا ... أو شيء ما لا يترجم

PS Jak w kojocie wyrównać do prawej? :>

Mimo, że znam bardzo dobrze angielski, oraz władam podstawami innych języków uważam, że to jest forum polskie i należy pisać po polsku. Są inne fora, gdzie pisze się po angielsku czy w innych językach. Ew. umieszczać posty w dwóch językach, miejmy jakąś godność jako Patrioci.

Co do meritum. Kiedyś coś podobnego pisałem. Po południu, lub wieczorem zerknę i postaram się wyrazić opinię.

To co o kodzie?

Proszę, prokomentujcie, co mogę polepszyć w swoim kodzie.

Klasy wyglądają na w miarę dobrze zaprojektowane, trudno wgryźć się w twój tok myślenia tylko po deklaracji klas bez komentarza czy jakiejkolwiek dokumentacji. Dodatkowo w ten typ gier nie grywam i mam dość mgliste pojęcie o budowie takiej gry.
Rzeczy, które rzuciły mi się w oczy i wydają się dla mnie nadmiarowe:
W klasie TParagraphList po co osobne właściwości EmptyParagraphs, NonEmptyParagraphs, NoChoicesParagraphs, OneChoiceParagraphs, TwoChoicesParagraphs, ThreeChoicesParagraphs, ManyChoicesParagraphs Nie lepiej dodać jedną metodę, która przefiltruje wszystkie dostępne i zwróci tylko te o określonej ilości wyborów? O ile w ogóle będzie to potrzebne.
Kolejna sprawa IMHO używanie specjalnych metod dla akcesorów read i write tak jak w klasie TParagraph ma sens jeśli dodatkowo będziemy zmieniać daną wartość lub sprawdzać zakres itp. jeśli właściwości odwołują się do pól bezpośrednio niepotrzebnie wydłuża to cały kod.

Wielkię dziękuję, szopenfx. Pan nawet nie wyobraże, jaka cenna ta uwaga.

1. Ciała metod w załącznikach.
2. http://editthis.info/create_your_own_story/Main_Page da pojęcie o takim typie gier.
3. Planuję potem dodać klasę z filtrami.
4. Tak,

read

i write

 bylo użyto za dużo. Borykam się z lenistwem, aby sprosćić deklaracje.

Przeglądnąłem kod i tak na szybko:

• w różnych metodach tworzysz lokalne obiekty i je zwalniasz - przydałoby się zabezpieczyć kod blokami Try Finally, np. tu:

procedure TParagraph.ReplaceLinks;
var
  g: Integer;
  pe: TPerlRegEx;
begin
  pe := TPerlRegEx.Create;
  pe.Options := [preCaseLess];
  pe.Subject := ParagraphText.Text;
  for g := 0 to Links.Count - 1 do
  begin
    pe.RegEx := '\[\[link ' + IntToStr(g + 1) + '\]\]';
    pe.Replacement := '<a href="' + IntToStr(Links[g].Target.Number)
      + '.xhtml">';
    pe.ReplaceAll;
  end;
  pe.RegEx := '\[\[end link\]\]';
  pe.Replacement := '</a>';
  pe.ReplaceAll;
  ParagraphText.Text := pe.Subject;
  pe.Free;
end;

w metodzie TParagraphList.Shuffle także by się taki blok przydał - zwiększy czytelność kodu i zabezpieczy przed ewentualnymi wyciekami pamięci;

• niektóre metody funkcyjne niepotrzebnie korzystają ze zmiennych lokalnych - można wykorzystać niejawną zmienną Result, np. tu:

function TParagraphList.Add: TParagraph;
var
  t: TParagraph;
begin
  t := TParagraph.Create;
  Add(t);
  Result := t;
end;

zmienna t nie jest potrzebna:

function TParagraphList.Add: TParagraph;
begin
  Result := TParagraph.Create;
  Add(Result);
end;

• nazewnictwo zmiennych nie jest najlepsze - pełno jest nic nie mówiących, jednoliterowych identyfikatorów; To nie jest dobra praktyka, bo jeśli się nie przeanalizuje danego kodu, to nie wiadomo do czego dana jednoliterowa zmienna służy; Niektórzy (w tym ja) widzą zaletę stosowania notacji węgierskiej - jeśli chcesz, zapoznaj się z tematem klikając w podany link;
• gdzieniegdzie wykorzystujesz inkrementację za pomocą operatora +, zamiast procedury Inc:

function TParagraphList.LinksCount: Integer;
var
  w: TParagraph;
begin
  Result := 0;
  for w in Self do
    Result := Result + w.Links.Count;
end;

można zamienić na:

function TParagraphList.LinksCount: Integer;
var
  w: TParagraph;
begin
  Result := 0;
  for w in Self do
    Inc(Result, w.Links.Count);
end;

Optymalizator powinien sobie z tym poradzić, ale i tak warto stosować tę procedurę (Dec także);

• metoda TParagraphLinkList.MoveLinkDown jest pusta - pasowało by coś w niej zapisać :]
• stosujesz różny styl nazewnictwa parametrów w metodach - niektóre mają prefiks A, a inne nie; Do tego niektóre piszesz z małej litery, a niektóre z dużej - wybierz jeden styl i stosuj go w całym kodzie, a zwiększysz czytelność kodu;
• podobnie z nawiasami - według mnie dobrą praktyką jest zapis pustych nazwiasów przy deklaracjach czy w miejscach używania metod, któe nie posiadają parametrów; Dzięki temu ad razu widać gdzie się wywołuje metodę, a gdzie korzysta z parametru czy zmiennej;
  Na początek to tyle, być może coś by się jeszcze znalazło, ale nie chcę wyszukiwać na siłę.

Szczerze przepraszam, ale pańska informacja stosuje się bardziej do stylu, natomiast chciałbym nauczyć się informacji o refaktoringach i design patternach, które można bylo by zastosować.