Pierwszy program w portfolio

Witam
Warunku w pętli while nie poprawiłem, ale za to dołożyłem kilka innych funkcji, zlikwidowałem jak i dodałem niektóre komentarze. Na pierwszym miejscu postawiłem na prawidłowe działanie procedur jak i funkcji. Wszystkie je przetestowałem i wszystkie działają prawidłowo. Błędów w testach żadnych nie zauważyłem. Moim zdaniem gdy wszystko działa to wtedy można pomyśleć nad rozbiciem "skomplikowanego kodu" występującego w niektórych miejscach programu.
Pozdrawiam

Ps. Jeśli ktoś wcześniej testował program to być może będzie musiał usunąć stary plik elementowy.

• PROGRAM8.PAS (0,03 MB) - ściągnięć: 13

Program po liftingu.

• PROGRAM8.PAS (0,03 MB) - ściągnięć: 8

@Adept123: jeszcze sporo masz do poprawienia. ;)

Masz bardzo dziwny sposób tworzenia wcięć, wyrównując zagnieżdżone linijki do końca pierwszego słowa nagłówka bloku. Nie rób tak – stosuj wcięcie o jednolitym rozmiarze, dokładnie dwóch znaków. Użyj wbudowanego w środowisko narzędzia do formatowania i zobacz jak drastycznie zmieni się Twój kod.

Nie stosujesz się do przyjętej konwencji nazewnictwa elementów kodu – rzadko kiedy używasz dużych liter, a to sprawia, że kod trudniej się analizuje. Styl PascalCase to podstawa, jeśli o identyfikatory chodzi, tak samo jak przyjęty system prefiksów dla różnych typów identyfikatorów. Poza tym mieszasz identyfikatory angielskie z polskimi.

Kolejna rzecz to interpunkcja i nawiasy. W przypadku bezparametrowych podprogramów nie zapisujesz pustych nawiasów, przez co gołym okiem nie da się odróżnić wywołania funkcji od użycia np. zmiennej. Co prawda składnia pozwala na pominięcie nawiasów w takim przypadku, jednak ich stosowanie poprawia czytelność kodu. Inny problem to pomijanie średnika po ostatniej instrukcji w danym bloku kodu. To też jest dopuszczalne, jednak niezalecane – w końcu jest to pozostałość po prehistorycznych kompilatorach, które braku średnika wymagały.

Poza tym często mieszasz logikę z wyświetlaniem i pobieraniem danych z klawiatury, a także unikasz używania parametrów funkcji i zamiast tego korzystasz ze zmiennych globalnych, a to prędzej czy później nie wyjdzie na dobre.

No i nie rozumiem co tam na górze robi moduł Dos. Ten program jest przeznaczony dla DOS-a?

Jeśli o techniczne sprawy chodzi, to niektóre funkcje są strasznie długie i pasowałoby je podzielić na mniejsze. Tak po kolei:

if (telef[cyfra] in ['0'..'9']) or (((telef[cyfra] = '*') or (telef[cyfra] = '+')) and (cyfra = 1)) then

taką linijkę możesz napisać w ten sposób, skoro i tak ze zbiorów korzystasz:

if (telef[cyfra] in ['0'..'9']) or ((telef[cyfra] in ['*','+']) (and cyfra = 1)) then

Możesz też wyciągnąć sprawdzanie pierwszego znaku przed pętlę, a tę indeksować od drugiego znaku, już bez dodatkowej instrukcji warunkowej.

function Poprawna_data(d,m,r: integer): boolean;
var
   skrocony_rok,wiek: integer;
begin
     Poprawna_data := false;

     skrocony_rok := r mod 100;
     wiek := r div 100;

     if (d in [1..31]) and (m in [1..12])
     and (((wiek = 19) and (skrocony_rok in [50..99])) or  ((wiek = 20) and (skrocony_rok in [0,1])))
     then
         if ((m = 1) and (d in [1..31])) or
            ((m = 2) and (d in [1..29])) or
            ((m = 3) and (d in [1..31])) or
            ((m = 5) and (d in [1..31])) or
            ((m = 7) and (d in [1..31])) or
            ((m = 8) and (d in [1..31])) or
            ((m = 10) and (d in [1..31])) or
            ((m = 12) and (d in [1..31])) or
            ((m <> 2) and (d in [1..30]))
         then Poprawna_data := true
end;

Ta funkcja jest zbędna, bo możesz sprawdzić poprawność składowych daty za pomocą funkcji TryEncodeDate.

Procedura Czytaj i końcowy fragment:

if obce[1] <> '' then
  begin
       writeln('j©zyk drugi: '); obce[2] := '';
       readln(obce[2]);

       if obce[2] <> '' then
         begin
              writeln('j©zyk trzeci: '); obce[3] := '';
              readln(obce[3]);

              if obce[3] <> '' then
                begin
                     writeln('j©zyk czwarty: '); obce[4] := '';
                     readln(obce[4])
                end
         end
  end;

Wczytujesz dane do tablicy, więc powinieneś skorzystać z pętli, a literały łańcuchowe wrzucić do stałej tablicy.

if znaleziono = true then

Hmm…

if znaleziono = true and not false then

Co Ty na to? ;)

Warunek wymaga wartości logicznej, a zmienna znaleziono zawiera wartość logiczną – porównanie zbędne.

To tak z grubsza, bo wszystkiego wymienić nie dam rady ze względu na potrzebny czas do analizy całości.