Uwagi

[MarcinCiura]

1. Stałe w shared.py WIELKIMI_LTERAMI. Ponadto nie rozumiem skrótów bgg, igg i xgg (vide 2. rozdział Poradnika).
2. dbmenager.py lepiej nazwać dbmanager.py.
3. W tej i poniższych funkcjach użyć

   SELECT exists(SELECT * FROM Products WHERE product_name = ?)

4. Ponadto wyszukać wszelkie wystąpienia .format( w tym pliku i zastąpić je pytajnikami.
5. Części as m w import main as m i as s w import shared as s to przegięcie (znowu punkt o skracaniu nazw w 2. rozdziale).
6. Ponadto nie rozumiem importowania modułu main w innych modułach. Coś stoi na głowie.

[Intensivee]

Dostosowałem projekt do Pana zaleceń oraz poprawiłem i dopracowałem inne aspekty. https://github.com/Intensivee/shop-project-gui-tkinter

Z wyrazami szacunku, Aleksander Kowalczyk.

wt., 19 maj 2020 o 10:52 Marcin Ciura notifications@github.com napisał(a):

1. Stałe w shared.py WIELKIMI_LTERAMI. Ponadto nie rozumiem skrótów bgg, igg i xgg (vide 2. rozdział Poradnika).
2. dbmenager.py lepiej nazwać dbmanager.py.
3. W tej i poniższych funkcjach https://github.com/sunrisemystery/car-dealer-manager-python/blob/f2fe99f9f45b5da5972b570296e4746c447a6ea7/customersRegister.py#L39 użyć

SELECT exists(SELECT * FROM Products WHERE product_name = ?)

1. Ponadto wyszukać wszelkie wystąpienia .format( w tym pliku i zastąpić je pytajnikami.
2. Części as m w import main as m i as s w import shared as s to przegięcie (znowu punkt o skracaniu nazw w 2. rozdziale).
3. Ponadto nie rozumiem importowania modułu main w innych modułach. Coś stoi na głowie.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Intensivee/shop-project-gui-tkinter/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/APJYJTGPKHIVN676XGYZAGDRSJCERANCNFSM4NEZSD7A .

[MarcinCiura]

1. Używać pylinta.
2. globals.py to licha nazwa pliku, bo przesłania wbudowaną funkcję globals(). Naprawdę wolałbym się nie zajmować rzeczami, którymi może się zająć pylint.
3. Stałe PRODUCT_COLUMNS itp. powtarzające się w plikach powinny być zebrane w zbiorczym module (globals po zmianie nazwy).
4. O porównaniach z '' i ()` jest nowy punkt w 2. rozdziale.
5. Nie używamy skrótów. Acc to może być też acceleration, accept, access i 200 innych wyrazów (2. rozdział).
6. MIN_PASSWORD_LENGTH (2. rozdział).
7. Z red zrobić stałą ERROR_FOREGROUND, ALERT_FOREGROUND lub tp.
8. text=name.
9. Skąd tu tyle nawiasów i dlaczego w pliku pani Gajzler widziałem tę samą osobliwość? To drugie pytanie nie jet retoryczne; proszę o odpowiedź.
10. Lepiej zwracać listę lub None niż listę lub False.
11. SELECT * zwraca kolumny w liczbie i kolejności, których trzeba szukać w kodzie tworzącym tabelę. Lepsze jest jawne wymienianie kolumn.
12. Tu i przy innych zmianach zdecydowanie conn.commit().
13. Mam wrażenie, że źle Pan używa with conn:. Moim zdaniem tam w środku powinno być pobieranie nowego kursora z conn, inaczej z rollbacku nici. Może też Pan skorzystać z tego, że metody kursora działają też na połączeniach i pisać

    with conn:
    conn.execute('...')
    ...

14. Nazwa conn mi się nie podoba mimo tego, że oficjalna dokumentacja jej używa (2. rozdział, o skrótach). A co mówi pylint na c? Naprawdę wolałbym nie zajmować się rzeczami, którymi on się może zająć.
15. Bez przecinka przed "that".

[MarcinCiura]

13. Dzisiaj już nie jestem taki pewien, że jest źle. Gdyby Pan znalazł argumenty za albo przeciw, mógłbym się czegoś nauczyć.

[Intensivee]

Dziękuje za cenne uwagi, wprowadziłem poprawki.

Jeżeli chodzi o Pana punkty: 3.PRODUCT_COLUMNS ma taką samą nazwę w 2 plikach ale te dwie stałe różnią się nieznaczenie. Czy w takim przypadku konieczne jest wstawienie ich do globals(obecnie my_config) pod różnymi nazwami?

9.Na początku pomyślałem że to sqlitestudio wygenerował taki kod (kilkukrotnie poprawiałem w nim tabelki), ale nie. Tabele tworzyłem na początku semestru w lutym i jakkolwiek głupio to brzmi nie mam pojęcia skąd te nawiasy i czemu Pani Gajzler też takie ma.

12.Znalazłem taki przypis w dokumentacji https://docs.python.org/2/library/sqlite3.html#using-the-connection-as-a-context-manager Od teraz we wszystkich funkcjach w których wykonuje jakąś zmianę w bazie, robię to bezpośrednio na połączeniu, natomiast przy Selectach wykorzystuje kursor. Czy przy takiej zmianie dalej lepszym rozwiązaniem będzie connection.commit()?

13.Kursor tworze osobno w każdej funkcji( w każdym połączeniu tego wymagającym) zgodnie z Pana pierwszą myślą, jest to z pewnością czytelniejsze i pewniejsze rozwiązanie niż poprzednie.

I)Czy niektóre linijki mogą pozostać nieznacznie dłuższe niż 100 znaków, (101-107) czy raczej trzymać się konwencji?

[MarcinCiura]

Dziękuję za wiadomość. ad 3. Jak coś jest potrzebne w jednym module, to nie musi być widoczne dla innych modułów. ad 12. W takim razie connection.commit() wydaje się zbędne. ad I. Jeśli GitHub je wyświetla tak, że nie trzeba nic przewijać w bok, to mogą zostać.

[Intensivee]

Dodałem do projektu testy

[MarcinCiura]

Dziękuję za wiadomość. W testach posortować importy i — co ważniejsze — dodać na końcu magiczne dwa wiersze, bez których się nie uruchomią (3. rozdział; jak Pan do tej pory sprawdzał, czy działają?)

[Intensivee]

Dziękuje za odpowiedź. Poprawione. Wcześniej uruchamiałem identycznie jak po dodaniu if name (run w kompilatorze). Możliwe że ratował mnie pycharm i mimo tego niedopatrzenia wiedział co zrobić z takim kodem.

[MarcinCiura]

1. i to zła nazwa (2. rozdział).
2. Nazwać (2. rozdział).
3. Tutaj może być from tkinter import messagebox — krócej.
4. Przenieść do nowego wiersza za .format( (2. rozdział). Nadto zamiast potrójnych cudzysłowów lepiej by wyglądały dwa wiersze z pojedynczymi, za to dobrze wciętymi (same się skleją, jak są koło siebie).
5. Patrz punkt 3. w pierwszych uwagach. Nadto nieładnie raz zwracać False, a raz napisy. Do tego tutaj lepiej zdefiniować stałe CUSTOMER_ABSENT, CUSTOMER_LOGIN, CUSTOMER_EMAIL = range(3) (2. rozdział).
6. Tu i niżej wcięcia licho wyglądają. Sugeruję wcięcia wg szablonu z 3. rozdziału.
7. O, znalazłem do punktu 2. Tylko zmienić na duże litery, bo to stała (2. rozdział).

[Intensivee]

Dziękuje, projekt został poprawiony.

[MarcinCiura]

Dziękuję za poprawki.

1. W testach: TestProductsMenu na ProductsMenuTest itd. (3. rozdział).
2. Jak Pan raz zwraca cóś, drugi raz True, a trzeci False, to potem potrzebuje Pan dziwnych rozwiązań z pętlami przy używaniu. Ta funkcja jest zła, bo w połowie przypadków nie ma nic wspólnego z usuwaniem klientów. Rozbić na dwie, bez magicznych parametrów. To samo wszędzie.
3. desc w SQL-u to przeciwieństwo ASC przy ORDER BY. Nie używamy skrótów (2. rozdział).

[Intensivee]

Dziękuje, poprawione.

[MarcinCiura]

LGTM. Gratuluję, jest Pan ósmy.