Bogen08 / Battleships

Python Battleships Online
0 stars 0 forks source link

Uwagi #1

Open MarcinCiura opened 4 years ago

MarcinCiura commented 4 years ago
  1. Okrety_Projekt.p przemianować na coś z .py na końcu.
  2. requirements.txt wyrzucić (1. rozdział Poradnika).
  3. Przepuścić całość przez pylint (2. rozdział). W szczególności: używać mówiących, a nie jednoliterowych nazw zmiennych (2. rozdział), lepiej używać 4 spacji na wcięcia, a nie 8, nie używać skrótow zrozumiałych tylko dla Pana (self.mapp, self.mapu itp.) (2. rozdział).
  4. Przeczytać punkt z szablonem głównego modułu w 3. rozdziale i zastosować go do Okrety_Projekt.p.
  5. Dopisać testy do czegokolwiek.
  6. Funkcjonalność — zgodnie z konsultacjami: wykluczyć stykanie się statków bokami i rogami, pobierać adres komputera przeciwnika od użytkownika.
Bogen08 commented 4 years ago

Dokonano poprawek według zaleceń ( format pliku, struktury, nazwy zmiennych, funkcjonalności ), dodano testy oraz usunięto plik requirements.txt

MarcinCiura commented 4 years ago

Dziękuję za poprawki. Oto kolejne uwagi.

  1. Magiczne stałe 1, 2, 10, 1024, 'x', 'o', 't' i inne ponazywać (dużymi literami; 2 rozdział) i używać tych nazw.
  2. W get_shp() brakuje i, a nazwy get_hp() wcale nie rozumiem.
  3. Tu dać _ zamiast canoname i wyrzucić del poniżej. To samo niżej.
  4. Tu użyć for char in string.ascii_lowercase[:BOARD_SIZE]: (wyżej dopisać import string). To samo niżej.
  5. Tu użyć enumerate(map1, 1) (3. rozdział).
  6. Tu zamast 65 użyć ord('A').
  7. Tu użyć not 0 <= ... < BOARD_SIZE.
  8. Podobnie w miejscach, gdzie jest <= 9, używać < BOARD_SIZE.
Bogen08 commented 4 years ago

Wszystkie poprawki zrobione ( stałe, nazwy funkcji, poprawki w strukturze poszczególnych funkcji)

MarcinCiura commented 4 years ago

Dziękuję za email przypominający.

  1. Nie wiem, co to chr(92). Czemu nie może być print('\\', end =' ')?
  2. BOARD_SIZE.
  3. Z tego '-' zrobić nazwaną stałą.
  4. if left: (3. rozdział; pylint to chyba zgłasza, czemu Pan go nie słucha?)
  5. Rozbić na dwa wiersze.
  6. okrety_testy.py przemianować na okrety_test.py i powyrzucać zbędne docstringi z metod (3. rozdział).
  7. Tu pisać bez \\ (3. rozdział):
    spam = ('abcd'
        'efgh')
Bogen08 commented 4 years ago

Wszystkie poprawki zrobione ( zmienne i stała, import, usunięto niepotrzebne docstringi i poprawiono stringi dwulinikowe )

  1. if left: (3. rozdział; pylint to chyba zgłasza, czemu Pan go nie słucha?)

Link wskazuje na linikę zawierającą: if left is True: , co jest zmianą wprowadzoną dodatkowo przy okazji poprzednich poprawek dla wszystkich zmiennych kierunkowych

MarcinCiura commented 4 years ago

Dziękuję za odpowiedź. Poniżej kolejna porcja uwag. Po ich poprawieniu albo będę miał drobne, kosmetyczne uwagi, albo od razu zaliczę Pana projekt.

  1. Literówka: ma być length.
  2. Ma być if left: (powtarzam zgłoszenie; 2. rozdział, nie 3. — moja pomyłka).
  3. Te stałe od 1 do 5 ponazywać.
  4. To się powtarza, być może z niewielkimi zmianami, kilka razy. Wyekstrahować do funkcji.
  5. Nie używać niezrozumiałych skrótów (2. rozdział).
  6. BOARD_SIZE - 1.
  7. if długa_rzecz in (STAŁA_1, STAŁA_2):
  8. Złe nazwy zmiennych i i j (2. rozdział; Pańskie i i j to nie liczniki pętli).
Bogen08 commented 4 years ago

Wszystkie poprawki zrobione ( nazwy, zmiany w strukturze, wyciągnięcie wprowadzania danych na pola do osobnej funkcji w miarę możliwości )

MarcinCiura commented 4 years ago

Nie używać magicznych napisów do przekazywania informacji (2. rozdział).

Bogen08 commented 4 years ago

Poprawione (zmienna kierunkowa w funkcji )

MarcinCiura commented 4 years ago

LGTM, ale dlaczego to nie może być CHOICE_LEFT, ..., CHOICE_BACK = range(5), jak w przykładzie z Poradnika?

Bogen08 commented 4 years ago

Poprawione, nie pomyślałem o tej opcji przy tworzeniu tych stałych