Sprawdzenie kodu - kolejka

0

Mam program realizujący kolejkę:

 program Projekt;

{$mode objfpc}{$H+}

uses
  {$IFDEF UNIX}{$IFDEF UseCThreads}
  cthreads,
  {$ENDIF}{$ENDIF}
  Classes
  { you can add units after this };

type TZesp = class
  Re, Im: real;
  constructor Create(a, b: real);
end;

constructor TZesp.Create(a, b: real);
begin
        Re := a;
        Im := b;
end;

type PKolejka = ^TKolejka;
    TKolejka = record
    Number: TZesp;
    Next: PKolejka;
    end;

type KKolejka = class
  Head: PKolejka;
  Tail: PKolejka;
  constructor Create;
  procedure Push(e: TZesp);
  function Pop:TZesp;
  procedure Show;
  procedure Remove;
  procedure RemoveAll;
end;

constructor KKolejka.Create;
begin
        Head := nil;
        Tail := nil;
end;

procedure KKolejka.Push(e: TZesp); //dodawanie elmentu do kolejki
var Nowy: PKolejka;
begin
        New(Nowy);
        Nowy^.Number := TZesp.Create(e.Re, e.Im);
        Nowy^.Next := nil;
        if Head = nil then  //<---------------------------------------------------------------
        begin
          Head := Nowy;
          Tail := Nowy;
        end
        else begin
          Tail^.Next := Nowy;
          Tail := Nowy;
        end;
end;

function KKolejka.Pop: TZesp;  //pobieranie elementu z kolejki
var Temp: PKolejka;
begin
        if Head = nil then
        begin
          writeln('Kolejka pusta');
        end
        else begin
          Result := Head^.Number;
          Temp := Head;
          Head := Head^.Next;
          Dispose(Temp);
        end;
end;

procedure KKolejka.Show;  //wypisanie kolejki
var Temp: PKolejka;
begin
        if Head = nil then writeln('Kolejka pusta')
        else Temp := Head;
        while Temp <> nil do
        begin
             writeln(Temp^.Number.Re,' + ',Temp^.Number.Im,'j');
             Temp := Temp^.Next;
        end;
end;

procedure KKolejka.RemoveAll; //usuniecie kolejki
var Temp: PKolejka;
begin
        while Head <> nil do
        begin
             Temp := Head^.Next;
             Dispose(Head);
             Head := Temp;
        end;
end;

procedure KKolejka.Remove; //usuniecie jednego elementu
var Temp: PKolejka;
begin
        if Head <> nil then
        begin
          Temp := Head^.Next;
          Dispose(Head);
          Head := Temp;
        end;
end;

var
  kolejka: KKolejka;
  liczba: TZesp;

begin
  kolejka.Create;
  kolejka.Push(TZesp.Create(3.5,4.5));
  kolejka.Push(TZesp.Create(3,5));
  kolejka.Push(TZesp.Create(35,4));
  kolejka.Show;
  liczba := kolejka.Pop;
  kolejka.Show;
  readln;
end.

#Przy uruchomieniu, w zaznaczonym miejscu wyrzuca wyjątek External: SIGSEGV. Debugowanie w Lazarusie jest trochę toporne, ale udało mi się dowiedzieć, że w momencie zwracania wyjątku, Head ma wartość Cannot access memory at address 0x8. Nie wiem co z tym zrobić.
#Czy taka realizacja kolejki jest poprawna?

2

poczytaj jak się tworzy instancję klasy bo to kolejka.Create; jest źle

0

Fakt. Zmieniłem na kolejka := KKolejka.Create;
Teraz pytanie 2.

2
  1. TZesp jako klasa mi się nie podoba bo wtedy traci sens cała lista na wskaźnikach
  2. Masz wycieki pamięci bo nigdzie nie zwalniasz TKolejka.Number
  3. RemoveAll powinno "korzystać" z Remove - nie bawimy się w kopiego pasta
  4. w metodzie Push niepotrzebnie tworzysz kolejną instancję klasy TZesp nie zwalniając nigdzie tej przekazanej do metody - kolejny wyciek pamięci
  5. w metodzie Pop jak kolejka jest pusta to by wypadało np. nil zwrócić albo rzucić błędem

Generalnie za dużo namieszałeś. Kolejka powinna być albo cała jako klasa albo cała jako rekordy i wskaźniki, inaczej to nie ma sensu i jak widać sprawia Ci duże problemy

0

Heh, program napisałem na wzór stosu podanego przez prowadzącego. Powiedzmy, że nie mam zaufania do jego programów i widać miałem dobre przeczucie.

  1. Jakie masz inne rozwiązanie?
  2. Czyli trzeba najpierw Dispose(Temp^.Number);, a później Dispose(Temp);?
  3. Ok
  4. Nowy^.Number := e;
if Head = nil then
        begin
          Result := nil;
          writeln('Kolejka pusta');
        end

Generalnie muszą być wskaźniki. Da się zrobić wskaźniki i klasę?

dodanie znacznika <code class="delphi"> - fp

0

@Przeszczep - a musisz koniecznie korzystać z klas, jeśli i tak wszystko robisz na wskaźnikach?

Deklaracja Twoich klas jest bardzo dziwna - nawet nie korzystasz z sekcji określających poziomy dostępu, tylko wszystko na kupę...

0

Czyli jak to najlepiej zrobić z użyciem wskaźników?

type PKolejka = ^TKolejka;
	TKolejka = record
	Re, Im: real;
	Next: PKolejka;
	end; 

Do tego funkcje, a Head i Tail tworzyć w programie głównym?

0

jeśli masz zadanie, że ma być i klasa i wskaźniki to jak zmienisz rekord na taki jak z powyższego postu i pozbędziesz się klasy TZesp to rozwiąże to większość moich uwag. Klasa opakowująca kolejkę z metodami pop i push jest OK tylko ta nieszczęsna TZesp psuje wszystko

0

push i pop to raczej określenia związane ze stosem - do kolejki przyjęło się używać enqueue i dequeue.

Czyli pop i push powinny zwracać/pobierać rekord? Czy do push podawać zmienne i tworzyć nowy element?

Niech przyjmuje/oddaje pointer na rekord;

Poczytaj sobie w ogóle jak wygląda kolejka i jak się jej używa, bo mam wrażenie, że nie do końca wiesz z czym się to je.

0

Nowa wersja:

program Projekt;

{$mode objfpc}{$H+}

uses
  {$IFDEF UNIX}{$IFDEF UseCThreads}
  cthreads,
  {$ENDIF}{$ENDIF}
  Classes, crt
  { you can add units after this };

type PKolejka = ^TKolejka;
    TKolejka = record
    Re, Im: real;
    Next: PKolejka;
    end;

type KKolejka = class     // klasa kolejki
  private
         Head: PKolejka;
         Tail: PKolejka;
  public
        constructor Create;
        destructor Destroy;
        procedure Enqueue(e: PKolejka);
        function Dequeue: PKolejka;
        procedure Show;
        procedure Remove;
        procedure RemoveAll;
        function IsEmpty: boolean;
end;

constructor KKolejka.Create;   // konstruktor
begin
        Head := nil;
        Tail := nil;
end;

destructor KKolejka.Destroy;   // destruktor
begin
        RemoveAll;
end;

procedure KKolejka.Enqueue(e: PKolejka); // dodawanie elmentu do kolejki
var Nowy: PKolejka;
begin
        New(Nowy);
        Nowy^.Re := e^.Re;
        Nowy^.Im := e^.Im;
        Nowy^.Next := nil;
        if Head = nil then
        begin
          Head := Nowy;
          Tail := Nowy;
        end
        else begin
          Tail^.Next := Nowy;
          Tail := Nowy;
        end;
end;

function KKolejka.Dequeue: PKolejka;  // pobieranie elementu z kolejki
var Temp: PKolejka;
begin
        if Head = nil then
        begin
          Result := nil;
          writeln('Kolejka pusta');
        end
        else begin
          Result := Head;
          Temp := Head;
          Head := Head^.Next;
          Dispose(Temp);
        end;
end;

procedure KKolejka.Show;  // wypisanie kolejki
var Temp: PKolejka;
begin
        Temp := Head;
        if Temp = nil then writeln('Kolejka pusta');
        while Temp <> nil do
        begin
             writeln(Temp^.Re:6:2,' + ',Temp^.Im:6:2,'j');
             Temp := Temp^.Next;
        end;
end;

procedure KKolejka.RemoveAll; // usuniecie calej kolejki
begin
        while Head <> nil do
        begin
             Remove();
        end;
end;

procedure KKolejka.Remove; // usuniecie jednego elementu
var Temp: PKolejka;
begin
        if Head <> nil then
        begin
          Temp := Head^.Next;
          Dispose(Head);
          Head := Temp;
        end;
end;

function KKolejka.IsEmpty: boolean;  // czy pusta
begin
        if Head = nil then Result := True
        else Result := False;
end;

procedure FillE(var e: PKolejka; Re, Im: real);
begin
        e^.Re := Re;
        e^.Im := Im;
end;

function Losuj: real;
begin
        Result := (random(20000)-10000)/100;
end;

var
  kolejka: KKolejka;
  element: PKolejka;
  i: integer;

begin
  Randomize;
  kolejka := KKolejka.Create;
  New(element);
  for i := 1 to 10 do
  begin
       FillE(element, Losuj, Losuj);
       kolejka.Enqueue(element);
  end;
  repeat
    begin
         kolejka.Show;
         element := kolejka.Dequeue;
         readln;
         clrscr;
    end;
  until kolejka.IsEmpty;
  kolejka.Show;
  Dispose(element);
  readln;
end.
1
furious programming napisał(a):

Czyli pop i push powinny zwracać/pobierać rekord? Czy do push podawać zmienne i tworzyć nowy element?

Niech przyjmuje/oddaje pointer na rekord;

ale po co? Po to się to opakowuje klasą aby się nie bawić podczas korzystania z klasy w tworzenie wskaźników. Przyjmować i zwracać powinien tylko i wyłącznie dane ważne z punktu widzenia tego, kto z klasy korzysta czyli Re i Im. Całą logiką wstawiania tego odpowiednio do kolejki powinna się zajmować klasa właśnie

0
abrakadaber napisał(a):

ale po co? Po to się to opakowuje klasą aby się nie bawić podczas korzystania z klasy w tworzenie wskaźników. Przyjmować i zwracać powinien tylko i wyłącznie dane ważne z punktu widzenia tego, kto z klasy korzysta czyli Re i Im. Całą logiką wstawiania tego odpowiednio do kolejki powinna się zajmować klasa właśnie

Po to była druga klasa, żeby ją pobierać i zwracać. Może ustalicie jakąś spójną wersję, bo nie wiem kogo słuchać.

0

Taka klasa jest w zasadzie bezużyteczna z tego powodu, że jak pewnego dnia będziesz chciał sobie w niej przechowywać jabłka to pozostanie ci skopiować całość na żywca i wkleić pod inną nazwą zmieniając wszystkie rekordy TZesp na TJablko lub wg. wersji @abrakadaber dodatkowo zmiana metod. Można zastosować typ Pointer, który "łyknie" każde dane lub obiekt TObject, ale wtedy użytkownik klasy ma nieprzyjemność rzutowania wszystkiego i traci się kontrolę nad typem. Przykładem takiej klasy jest TList.
Prawdopodobnie generyki by tutaj pomogły, ale nie korzystałem z nich jeszcze więc może ktoś inny w tej kwestii coś doradzi.
Stosując Tobject lub Pointer traci się w zasadzie całą zaletę list czyli to, że nie trzeba kopiować/usuwać/przesuwać dużych bloków pamięci w odróżnieniu od np. tablicy rekordów.
Przy tablicy wskaźników na rekordy jest to już mniejsza różnica bo nie musimy przesuwać fizycznie danych tylko wskaźniki.

1

@abrakadaber - ja wiem, że i tak źle i tak nie dobrze, dlatego poleciłem przekazywanie pointera; Ma to mniej więcej tyle samo wad i zalet co podawanie danych i wewnętrzne tworzenie elementów kolejki;

Idealnego rozwiązania pewnie nie ma (i elastycznego i wygodnego), dlatego aby pogodzić te dwie zalety wykombinowałbym coś w rodzaju jądra kolejki do zarządzania pamięcią oraz klasy-nakładki jako wygodnego interfejsu; Klasa jądra alokowałaby pamięć, przesuwała wskaźniki itd., a nakładka dawałaby wygodę przyjmując składniki struktury w parametrach metod, tworząc i wypełniając rekordy w wewnętrznych mechanizmach; Te dwie klasy zapewnią większą uniwersalność jądra kolejki oraz wykluczą konieczność zewnętrznego rzutowania przy różnych typach elementów kolejki (różnych kolejek korzystających z jednej klasy zarządzającej pamięcią);

To i tak nie jest idealne rozwiązanie, ale w tym kierunku bym eksperymentował;


@Przeszczep - teraz kilka wskazówek co do kodu;

Deklaracja klasy KKolejka jest trochę uboga i zbyt ogólna, dlatego że nie stosujesz sensownie sekcji określających poziom dostępu do metod; Metoda IsEmpty mogłaby być prywatna, do której dostęp uzyskiwałoby się przez publiczną właściwość read-only; To samo tyczy się metody RemoveAll, do której dostępu nie powinien mieć programista;

Publiczne powinny być metody Dequeue i Enqueue, zaś brakuje Ci właściwości Count, która mogłaby się przydać; Count powinna być połączona z prywatnym polem np. FCount, które było by inkrementowane po dodaniu elementu do kolejki, a dekrementowane po zdjęciu elementu z niej;

Kolejna sprawa - funkcja IsEmpty:

function KKolejka.IsEmpty: boolean;  // czy pusta
begin
        if Head = nil then Result := True
        else Result := False;
end;

nie wiem czy wiesz, ale każdy warunek i tak sprowadza się do wartości logicznej, więc możesz go skrócić do takiej postaci:

function KKolejka.IsEmpty(): Boolean;
begin
  Result := Head = nil;
end;

Nie wiem też czemu jeden element kolejki nazywasz kolejką - chodzi o typ TKolejka, który tak naprawdę zawiera dane jednego elementu, a nie całej kolejki; Zmień nazwę tego typu - mniej się będzie mylić.

1 użytkowników online, w tym zalogowanych: 0, gości: 1