Code review swojego własnego bardzo starego kodu

4

Trafiłem ostatnio na wpis Code Reviewing My Earliest Surviving Program, gdzie autor robi sobie code review swojego kodu sprzed 12 lat. W sumie ciekawy pomysł, prawda?

Ja osobiście mam archiwum sięgające do ~2000 roku, kiedy miałem naście lat, ale tamten kod napisany jest w QBASIC-u i średnio nadaje się nawet do zatykania toalety.

Ale znalazłem sobie inny wspaniały malutki i bezsensowny projekcik - implementacja "Gry w życie" w C# dla WinForms. Około 8 lat temu, chyba jakieś początki uczenia się C# "na poważnie", chyba pisane podczas sesji egzaminacyjnej. Wrzuciłem na https://github.com/ktos/Sterope. Więc może obejrzyjmy?

Podstawowa logika gry mieści się w https://github.com/ktos/Sterope/blob/master/SteropeW/GraWZycie.cs:

  • polskie nazewnictwo zmiennych;
  • brak komentowania publicznego API - znakomicie sobie opisałem czym są reguły ożywiania w komentarzu do zmiennej prywatnej je przechowującej, ale uruchamiając konstruktor nie widzę przecież tego komentarza;
  • reguły ożywiania i reguły zabijania raczej zrobiłbym teraz List<T> niż tablicą;
  • nieużywanie właściwości, na rzecz metod GetCośtam (ciekawe, pewnie wynik poprzedniego semestru pisania w Javie);
  • dwie klasy w jednym pliku;
  • zamiast LifeException raczej użyłbym ArgumentException;
  • niepotrzebny standardowy komentarz w LifeException.

Z kolei w samej implementacji WinForms https://github.com/ktos/Sterope/blob/master/SteropeW/Form1.cs:

  • nazewnictwo kontrolek!;
  • nazewnictwo metod!

I jeszcze jedno bardzo ważne pytanie - dlaczego wydajność tej aplikacji jest tak żałosna? (odpowiedź: bo DataGridView tak ma, bo nie ma DoubleBuffering, ale da się to załatwić: http://stackoverflow.com/a/1506066)

Co jeszcze byście powiedzieli?
Na który rok studiów to wygląda?
Kto jeszcze ogląda swój stary kod i się uśmiecha?

1

Kto jeszcze ogląda swój stary kod i się uśmiecha?

Chyba już dojrzałem, żeby pochwalić się swoją pierwszą grą z paru lat wstecz :) (pascal+grafika BGI)
Ogólnie o grze: lata stateczek, generują się linie z lasera które trzeba omijać + wypada zbierać pieniążki.
Code review sobie daruję.

PS. Pisane w TP7

PROGRAM Danaimus;
Uses Crt,graph;
Type
 xY=record
  x,y:integer;
 end;
 xYxY=record
  x1,y1,x2,y2:integer;
 end;
Var
wsk,OldWsk,Mon,OldMon:xY; Linia,StaraLinia:xYxY;
karta,tryb:Integer;
Licznik:array[1..5] of byte; hp,PoziomTrudnosci,kolorNad:byte;
button:char; Ilosc_Pkt: longint;
Rusz,Czy_Linia,Wzielo_hp,Czy_Mon,Cant_add:boolean;
PROCEDURE losujPol;
 begin
 Linia.x1:=random(565)+10;
 Linia.y1:=random(150);
 Linia.x2:=random(565)+10;
 Linia.y2:=random(150);
 IF ((Linia.x1>=0) and (Linia.x2>=575)) OR ((Linia.x1>=545) and (Linia.x2>=0)) THEN LosujPol;
end;
PROCEDURE LosujMon;
begin
 Mon.x:=random(505)+40;
 Mon.y:=random(100);
end;
PROCEDURE ShowMon1;
begin
 SetColor(yellow);
 Rectangle(Mon.x,Mon.y,Mon.x-3,Mon.y-3);
end;
PROCEDURE ShowMon2;
begin
 SetColor(BLACK);
 Rectangle(OldMon.x,OldMon.y,OldMon.x-3,OldMon.y-3);
end;
PROCEDURE GetOldMon;
begin
 OldMon.x:=Mon.x; OldMon.y:=Mon.y;
end;
PROCEDURE GrafikaGracza;
Begin
 SetLineStyle(0,0,3); SetColor(7);
 Line(wsk.x,wsk.y+20,wsk.x,wsk.y); Line(wsk.x+3,wsk.y+20,wsk.x+2,wsk.y+4); Line(wsk.x+6,wsk.y+18,wsk.x+4,wsk.y+12);
 Line(wsk.x-3,wsk.y+20,wsk.x-2,wsk.y+4); Line(wsk.x-6,wsk.y+18,wsk.x-4,wsk.y+12);
end;
PROCEDURE GrafikaGracza2; {Poprawnosc: jakies 45%(bo mogloby byc duzo szybsze)}
Begin
 SetColor(BLACK);
 Line(OldWsk.x,OldWsk.y+20,OldWsk.x,OldWsk.y); Line(OldWsk.x+3,OldWsk.y+20,OldWsk.x+2,OldWsk.y+4);
 Line(OldWsk.x+6,OldWsk.y+18,OldWsk.x+4,OldWsk.y+12);
 Line(OldWsk.x-3,OldWsk.y+20,OldWsk.x-2,OldWsk.y+4); Line(OldWsk.x-6,OldWsk.y+18,OldWsk.x-4,OldWsk.y+12);
end;
PROCEDURE PoczZm;
begin
 wsk.x:=200;
 wsk.y:=400;
end;
PROCEDURE WczytajPrzycisk;
Begin
 button:=UpCase(readkey);
 CASE button OF
  'W': wsk.y:=wsk.y-5;
  'S': wsk.y:=wsk.y+5;
  'A': wsk.x:=wsk.x-5;
  'D': wsk.x:=wsk.x+5;
 end;
 Rusz:=true;
End;
PROCEDURE HealthPointsStarts;
begin
 SetColor(RED);
 Circle(5,15,5);
 Circle(5,35,5);
 Circle(5,55,5);
end;
PROCEDURE kolizje;
begin
 IF wsk.y<150 THEN wsk.y:=wsk.y+5;
 IF wsk.y>450 THEN wsk.y:=wsk.y-5;
 IF wsk.x<50 THEN wsk.x:=wsk.x+5;
 IF wsk.x>535 THEN wsk.x:=wsk.x-5;
 IF Mon.y>500 THEN Czy_Mon:=false;
 IF (linia.y1>500) and (Linia.y2>500) THEN BEGIN Czy_Linia:=false; Inc(Ilosc_Pkt); END;
end;
PROCEDURE GetOldXY;
begin
 OldWsk.x:=wsk.x; OldWsk.y:=wsk.y;
end;
PROCEDURE GetOldLine;
begin
 StaraLinia.x1:=Linia.x1; StaraLinia.y1:=Linia.y1; StaraLinia.x2:=Linia.x2; StaraLinia.y2:=Linia.y2;
end;
PROCEDURE ZmniejszHp;
begin
 IF hp=1 THEN button:=chr(27);
 Dec(hp);
 Wzielo_hp:=true;
 SetColor(BLACK);
 CASE hp OF
 2: Circle(5,15,5);
 1: Circle(5,35,5);
 0: Circle(5,55,5);
 end;
end;
PROCEDURE gra;
begin
 button:='o';
 licznik[1]:=0; Licznik[5]:=0;
 Czy_linia:=false; Czy_Mon:=false; cant_add:=false;
 SetColor(RED);
 Licznik[3]:=0; Licznik[4]:=0;
 hp:=3;
 HealthPointsStarts;
 Randomize;
 REPEAT
  GetOldXY;
  IF keypressed THEN WczytajPrzycisk;
  kolizje;
  IF Czy_Mon THEN begin
                   IF cant_add=true THEN cant_add:=false;
                   GetOldMon;
                   ShowMon2;
                   IF licznik[3]=0 THEN Inc(Mon.y);
                   ShowMon1;
                  end Else begin
                            IF licznik[5]=30 THEN begin
                                                   GetOldMon; ShowMon2;
                                                   losujMon; Czy_Mon:=true; Licznik[5]:=0;
                                                  end Else Inc(Licznik[5]);
                           end;
  IF (GetPixel(wsk.x,wsk.y-1)=yellow) and (cant_add=false) THEN begin Inc(Ilosc_Pkt); Czy_Mon:=false; cant_add:=true; end;
  IF (GetPixel(wsk.x,wsk.y-1)=GREEN) and (Licznik[4]=0) THEN ZmniejszHp;
  IF Czy_Linia THEN begin
                     SetColor(Black);
                     GetOldLine;
                     Line(StaraLinia.x1,StaraLinia.y1,StaraLinia.x2,StaraLinia.y2);
                     IF licznik[3]=0 THEN begin Inc(Linia.y1); Inc(Linia.y2); end;
                     SetColor(GREEN);
                     Line(Linia.x1,Linia.y1,Linia.x2,Linia.y2);
                     end Else begin losujPol; Czy_Linia:=true; end;
  IF Rusz THEN begin
                GrafikaGracza2;
                Rusz:=false;
               end;
  GrafikaGracza;
  Inc(Licznik[3]);
  IF Wzielo_hp THEN begin
                     Inc(Licznik[4]);
                     IF Licznik[4]=30 THEN begin
                                            Wzielo_hp:=false;
                                            Licznik[4]:=0;
                                           end;
                    end;
  CASE PoziomTrudnosci OF
   1: BEGIN IF licznik[3]>=5 THEN licznik[3]:=0; end;
   2: BEGIN IF Licznik[3]>=1 THEN Licznik[3]:=0; end;
   3: BEGIN Licznik[3]:=0; end;
  end;
  Delay(1);
 UNTIL button=chr(27);
end;
PROCEDURE PrzygotowanieGry;
begin
 ClearDevice;
 OutTextXY(200,200,'Wczytywanie...');
 PoczZm;
 GrafikaGracza;
 ClearDevice;
 Delay(10);
 gra;
end;
PROCEDURE wybWmenu3(SelCol,NotSelCol,Selected:byte;n1x,n1y,n2x,n2y,n3x,n3y:word;s1,s2,s3:string);
begin
 CASE Selected OF
  1: SetColor(SelCol); 2: SetColor(NotSelCol); 3: SetColor(NotSelCol);
 end;
 OutTextXY(n1x,n1y,s1);
 CASE Selected OF
  1: SetColor(NotSelCol); 2: SetColor(SelCol); 3: SetColor(NotSelCol);
 end;
 OutTextXY(n2x,n2y,s2);
 CASE Selected OF
  1: SetColor(NotSelCol); 2: SetColor(NotSelCol); 3: SetColor(SelCol);
 end;
 OutTextXY(n3x,n3y,s3);
end;
PROCEDURE OutColorTextXY(x,y,color:integer; s2:string);
begin
 SetColor(color);
 OutTextXY(x,y,s2);
end;
PROCEDURE graphON;
begin
InitGraph(karta,tryb,'D:\Danaimus\BGI');
end;
PROCEDURE graphOFF;
begin
 closegraph;
end;
PROCEDURE logo1;
begin
 PoziomTrudnosci:=3;
 SetFillStyle(10,4); FloodFill(100,100,2); SetLineStyle(0,0,3);
 FOR Licznik[1]:=20 DOWNTO 0 DO begin
                                 Circle(130,200,Licznik[1]+50+18);
                                 SetColor(4);
                                 Circle(130,200,Licznik[1]+50+18-22);
                                 Circle(130,200,Licznik[1]+50+18-44);
                                 Circle(130,200,Licznik[1]+50+18-66);
                                 SetColor(15);
                                 Delay(30);
                                end;
 FOR Licznik[1]:=20 DOWNTO 0 DO begin
                                 Circle(100,200,Licznik[1]);
                                 Circle(160,200,Licznik[1]);
                                 Delay(20);
                                end;
 OutTextXY(320,240,'prezentuje...');
 Delay(3000);
end;
PROCEDURE Opcje;
begin
 SetColor(Green);
 OutTextXY(370,120,'Ustaw poziom trudnosci:');
 PoziomTrudnosci:=2;
 REPEAT
  wybWmenu3(3,1,licznik[1],380,159,380,176,380,192,'Ciapowaty','Wzburzony','Powalony');
  button:=readkey;
  Sound(2000); Delay(10); NoSound;
  CASE button OF
   'w': Dec(licznik[1]); 's': Inc(licznik[1]);
  end;
  IF licznik[1]<1 THEN licznik[1]:=3; IF licznik[1]>3 THEN licznik[1]:=1;
 UNTIL button=chr(13);
 CASE licznik[1] OF
  1: PoziomTrudnosci:=1;
  2: PoziomTrudnosci:=2;
  3: PoziomTrudnosci:=3;
 end;
end;
PROCEDURE logo2;
begin
 ClearDevice;
 Delay(50);
 SetFillStyle(3,8);
 FloodFill(100,100,2);
 SetTextStyle(1,0,4);
 OutColorTextXY(110,100,1,'D'); OutColorTextXY(130,100,2,'A'); OutColorTextXY(150,100,3,'N'); OutColorTextXY(170,100,4,'A');
 OutColorTextXY(190,100,5,'I'); OutColorTextXY(200,100,6,'M'); OutColorTextXY(223,100,7,'U'); OutColorTextXY(246,100,9,'S');
 Line(30,155,30,210); Line(115,155,115,210); Line(30,155,115,155);
 Line(30,210,115,210); Line(30,173,115,173); Line(30,192,115,192);
 SetTextStyle(2,0,4);
 licznik[1]:=1;
 REPEAT
  wybWmenu3(3,1,licznik[1],40,159,40,176,40,195,'Nowa Gra','Opcje','Wyjscie');
  button:=readkey;
  Sound(2000); Delay(10); NoSound;
  CASE button OF
   'w': Dec(licznik[1]); 's': Inc(licznik[1]);
  end;
  IF licznik[1]<1 THEN licznik[1]:=3; IF licznik[1]>3 THEN licznik[1]:=1;
 UNTIL button=chr(13);
 CASE licznik[1] OF
  1: PrzygotowanieGry;
  2: begin Opcje; logo2; end;
  3: Halt;
 end;
end;
Begin
 {$i-}
 graphON;
 logo1;
 logo2;
 GraphOFF;
 GotoXY(15,15);
 Write('Twoja ilosc zdobytych punkt¢w to: ',Ilosc_Pkt);
 Delay(2000);
 GotoXY(15,17);
 Write('...Wcisnik przycisk...');
 readkey;
 {$i+}
End.

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