Wątek przeniesiony 2016-05-14 16:46 z Delphi i Pascal przez furious programming.

Sortowanie listy jednokierunkowej metodą wybierania.

0

Witam. Mam problem z sortowaniem listy jednokierunkowej. Mógłby mnie ktoś oświecić co mam źle?

Edit: po poprawkach wydaje mi się, że wszystko działa. Mógłby mi ktoś ewentualnie powiedzieć co można poprawić/ zrobić lepiej?

 program dadada;
uses SysUtils,Crt;
type plista=^tlista;
  tlista=record
    imie:string[15];
    indeks:integer;
    nast:plista;
            end;
  var
    wyb,i:integer;
    pocz,kon:plista;

procedure dodaj (var pocz,kon:plista);
var
nowy:plista;

begin
clrscr;
randomize;

new (nowy);
writeln ('Imie i indeks');
readln (nowy^.imie);
nowy^.indeks:=random (16)+1;

if pocz=nil then begin
 pocz:=nowy; kon:=nowy; kon^.nast:=nil; end

else

if pocz<>nil then begin
kon^.nast:=nowy; kon:=nowy; kon^.nast:=nil; end;

writeln ('Gotowe');
end;


procedure wyswietl (pocz,kon:plista);
var
nowy:plista;

begin
clrscr;
nowy:=pocz;

while (nowy<>nil) do begin
writeln (nowy^.imie);
writeln (nowy^.indeks);
writeln ('==========================');

nowy:=nowy^.nast;
end;

if nowy=nil then writeln ('KONIEC');

end;

procedure edytuj (var pocz:plista);
var
i:integer;
n:plista ;
begin
n:=pocz ;
writeln ('ktor element chesz edytowac');
readln (i);


while n<>nil do begin

      if n^.indeks=i then begin
      readln (n^.imie);
      readln (n^.indeks);

      end;
n:=n^.nast;
end;

end;


procedure zapis (pocz:plista);
var
plik:textfile;
el:plista;
begin
el:=pocz;
assignfile (plik,'kolejka.txt');
append (plik);

while (el<>nil) do begin
write (plik,'Imie:', 'el^.imie','el^.indeks','||');
el:=el^.nast;
end;

closefile (plik);
writeln ('GOTOWE');
end;

procedure usun_wyb (var pocz,kon:plista);
var
el:plista;
pop,del,temp:plista;
i:integer;
begin
writeln ('jaki indeks chcesz usunac?');
readln (i);
el:=pocz;

while el<>nil do begin

if el^.indeks=i then begin

   if (el=pocz) and (el<>nil) then begin
   del:=el;
   pocz:=el^.nast;
   el:=el^.nast;
   dispose (del);

   end else

   if (el=kon) then begin

   del:=kon;
   kon:=pop;
   kon^.nast:=nil;
   dispose (del);

   end else

   if (el^.indeks=i) and (el<>nil) then begin
del:=el;
pop^.nast:=el^.nast;
dispose (del);
el:=el^.nast;
end else
    el:=el^.nast;

end;
end;

end;

procedure ilosc (var i:integer);
var
v:plista;

begin
clrscr;
v:=pocz;
i:=0;

while v<>nil do begin
 inc (i);
 v:=v^.nast;
 end;

writeln ('Ilosc elementow ',i);
end;

procedure sortuj (var pocz:plista);
var
min,min2,min3,tmp:tlista;
el,pom,pom2,pom3,w,wmin:plista;
j,o:integer;

begin
el:=pocz;

ilosc (i);

while (el<>nil) do begin

min.imie:=el^.imie;
min.indeks:=el^.indeks;


w:=el^.nast;

             while w<>nil do begin

                   if w^.indeks<min.indeks then begin
                      min.indeks:=w^.indeks;
                      min.imie:=w^.imie;
                      wmin:=w;
                                 end;


                      w:=w^.nast;

             end;
    if min.indeks < el^.indeks then begin

       tmp.indeks := el^.indeks;
       tmp.imie:=el^.imie;

       el^.indeks := wmin^.indeks;
       el^.imie:=wmin^.imie;

       wmin^.indeks:=tmp.indeks;
       wmin^.imie:=tmp.imie;
     end;
     el := el^.nast;
   end;
 end;


begin
pocz:=nil;
kon:=nil;

repeat
writeln;
writeln;
writeln ('1.Dodaj. 2.Wyswietl. . 4.Sortuj. 6.Usun konkretny. 7. Edytuj. 8. Zapisz.');
readln (wyb);
case (wyb) of
1:dodaj (pocz,kon);
2:wyswietl (pocz,kon);
4:sortuj (pocz);
6:usun_wyb (pocz,kon);
7:edytuj (pocz);
8:zapis (pocz);


end
 until (wyb=0);
end.

1

Edit: po poprawkach wydaje mi się, że wszystko działa.

No to wydaje Ci się, czy działa prawidłowo?

Mógłby mi ktoś ewentualnie powiedzieć co można poprawić/ zrobić lepiej?

Wszystko należy poprawić i zrobić lepiej; Ten kod już gdzieś widziałem - zawiera takie same błędy:

var
    wyb,i:integer;
    pocz,kon:plista;  // pocz i kon
 
procedure dodaj (var pocz,kon:plista);  // pocz i kon
var
nowy:plista;
{...}

Znowu polskie identyfikatory wszystkiego i znowu nazwy zmiennych zdublowane z nazwami parametrów, do tego wszystko globalne; I tak jak w kodzie który kiedyś widziałem, znów procedury/funkcje operujące na liście, dodatkowo coś wyświetlają w konsoli i pobierają dane od użytkownika....

Tragedia - kod do kosza i pisz od nowa, tym razem z głową; Kod ma być po angielsku w całości, ma trzymać się przyjętej konwencji nazewnictwa, masz używać klawisza Shift i porządnie formatować kod, stosując jeden styl dla wcięć i pustych linii;

Kod w obecnej postaci jest bardzo nieczytelny i nikomu nie będzie się chciało go analizować.

0

**furious programming **
Dzięki za wytknięcie błędów. Jeżeli chodzi o parametry to faktycznie jest to straszny babol. Muszę popracować nad estetyką bo wygląda to fatalnie.
Szkoda, że do tej pory nikt nie pokazał mi jak ma wyglądać PRAWDZIWY kawał dobrego i estetycznego kodu...
Myślę, że temat można zamknąć.

1

Na sam początek zapamiętaj jak nazywać konkretne elementy kodu; Zamiast tego:

type plista=^tlista;
  tlista=record
    imie:string[15];
    indeks:integer;
    nast:plista;
            end;

uzyj tego:

type
  PListNode = ^TListNode;
  TListNode = record
    Name: String[15];
    Index: Integer;
    NextNode: PListNode;
  end;

Prawda, że ładniej? Jednolite wcięcia, tylko i wyłącznie angielskie identyfikatory, używany styl PascalCase, typy wskaźnikowe z prefiksem P a zwykłe z T; Nie wiem dlaczego ograniczasz łańcuch dla imienia, ale nie musisz tego robić; Chyba że używasz plików typowanych do zapisu listy, ale w tym bałaganie nie mogę tego znaleźć...

Pola o nazwach Name i Index mogą być w edytorze kolorowane jako słowa kluczowe; Faktycznie - składnia zawiera słowa kluczowe Name i Index, jendak w tym kontekście dozwolone jest użycie tych słów do nazwania pól rekordu (ogólnie jako identyfikatorów);
Aby nie musieć ciągle przekazywać dwóch wskaźników na głowę i ogon listy, zadeklaruj sobie dodatkowy typ rekordu, który będzie je grupował:

type
  TList = record
    Head: PListNode;
    Tail: PListNode;
  end;

Head to wskaźnik na pierwszy węzeł listy (głowę), a Tail na ostatni (ogon); Taki rekord może dodatkowo zawierać jeszcze jedno pole z liczbą węzłów listy (np. Count) - może to uprościć operacje na liście;

Główny blok kodu modułu powinien wyglądać tak:

var
  List: TList;
begin
  InitList(List);

  while True do
    ShowMenu();

    case ChooseOption() of
      1: MenuAddNode(List);
      2: MenuEditNode(List);
      3: MenuRemoveNode(List);
      4: MenuShowList(List);
      5: MenuSortList(List);
      6: MenuSaveList(List);
      7: Break;
    end;
  end;

  DisposeList(List);
end;

Jak widzisz kod ładnie podzielony na czytelne procedury i funkcje; InitList powinno inicjować listę List - nilować wskaźniki lub ładować listę z pliku (choć tu przydałaby się procedura LoadListFromFile, gdzieś wewnątrz InitList); Następnie jest nieskończona pętla While - nieskończona, dlatego że pętla działa dotąd aż użytkownik wybierze opcję 7; Opcja ta wywołuje Break i przerywa działanie pętli; W pętli ShowMenu czyści ekran i wyświetla menu, następnie funkcja ChooseOption pobiera od użytkownika numer opcji; Procedury z prefiksem Menu wykonują zgrupowane czynności (o tym za chwilę); Na końcu DisposeList zwalnia z pamęci wszystkie węzły listy, aby nie powodować wycieków pamięci;

Dla przykładu, procedura MenuAddNode powinna wyglądać tak:

procedure MenuAddNode(var AList: TList);
var
  strName: String;
  intIndex: Integer;
begin
  ClrScr();

  Write('Type name: ');
  ReadLn(strName);

  Write('Type index: ');
  ReadLn(intIndex);

  AddListNode(AList, strName, intIndex);
end;

Jak widzisz procedura ta wcale nie operuje na liście - jest jedynie wrapperem, pobierającym dane od użytkownika; Po pobraniu danych następuje wywołanie procedury AddListNode, która to dokonuje fizycznego dodania nowego węzła do listy List; Jej ciało powinno wyglądać tak:

procedure AddListNode(var AList: TList; const AName: String; AIndex: Integer);
var
  plnNew: PListNode;
begin
  New(plnNew);

  plnNew^.Name := AName;
  plnNew^.Index := AIndex;
  plnNew^.NextNode := nil;

  if AList.Head = nil then
  begin
    AList.Head := plnNew;
    AList.Tail := plnNew;
  end
  else
  begin
    AList.Tail^.NextNode := plnNew;
    AList.Tail := plnNew;
  end;
end;

Jak widzisz procedura bardzo prosta, robi tylko to na co wskazuje jej nazwa - dodaje nowy element do listy, na podstawie danych z parametrów AName i AIndex; Prefiks A oznacza argument i jest zalecany przez przyjętą konwencję nazewnictwa;

Z pozostałymi procedurami z prefiksami Menu zrób tak samo - one niech pobierają dane od użytkownika i coś wyświetlają na ekranie, a wewnątrz siebie niech wywołują konkretne procedury operujące na liście, podając im dane w parametrach; Dzięki temu kod będzie bardzo czytelny i bardzo łatwo będzie go rozwijać - wystarczy dodać kolejne procedurki i podpiąć je pod menu.

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