Mój program dla pisania CYOA :: 4programmers.net

shinkarom

2014-08-07T16:42:42+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

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?

shinkarom

2014-08-07T17:00:43+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

Takie są moje klasy głowne:

Kopiuj

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.

babubabu

2014-08-07T21:07:38+02:00

Rejestracja: dni
Ostatnio: dni
Lokalizacja: Łódź
Postów: 648

0

A ciała tych metod?

olesio

2014-08-07T21:34:55+02:00

Rejestracja: dni
Ostatnio: dni
Lokalizacja: Szczecin
Postów: 4191

0

@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 :)

flowCRANE

2014-08-08T00:41:52+02:00

Rejestracja: dni
Ostatnio: dni
Lokalizacja: Tuchów
Postów: 12269

1

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

szopenfx

2014-08-08T00:47:43+02:00

Rejestracja: dni
Ostatnio: dni

0

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

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

serge

2014-08-08T08:43:50+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 68

1

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ę.

shinkarom

2014-08-13T10:29:43+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

To co o kodzie?

shinkarom

2014-08-15T08:48:28+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

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

szopenfx

2014-08-15T12:52:56+02:00

Rejestracja: dni
Ostatnio: dni

0

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.

shinkarom

2014-08-15T14:10:40+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

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

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

Kopiuj

read

i write

Kopiuj

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

flowCRANE

2014-08-15T15:45:02+02:00

Rejestracja: dni
Ostatnio: dni
Lokalizacja: Tuchów
Postów: 12269

0

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:

Kopiuj

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:

Kopiuj

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

zmienna t nie jest potrzebna:

Kopiuj

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:

Kopiuj

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:

Kopiuj

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łę.

shinkarom

2014-08-26T12:19:13+02:00

Rejestracja: dni
Ostatnio: dni
Postów: 10

0

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ć.