• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Prośba o ocenę programu w Pythonie

VPS Starter Arubacloud
+2 głosów
367 wizyt
pytanie zadane 19 maja 2021 w Nasze projekty przez mattaha Użytkownik (690 p.)
Siema! Rzuci ktoś okiem na mój programik napisany w Pythonie?
Piszę program do nauki historii. Mam zamiar dokonać w nim kilku ulepszeń jak np. ogarnięcie formatu json zamiast pickle, zrobienie jakiegoś gui w tk lub curses, dodanie ascii artów.
Byłbym wdzięczny za code review.

https://dpaste.com/G8LMKFJJJ

2 odpowiedzi

+3 głosów
odpowiedź 19 maja 2021 przez Ehlert Ekspert (212,630 p.)
  1. Zrób repo na githubie
  2. Dlaczego nie ma type hintów crying
  3. Podziel kod na package'e oraz pliki. Jeden plik na 500 linijek słabo się czyta.
  4. Warning. The pickle module is not secure. Only unpickle data you trust.
    Także dobry pomysł z wywaleniem ogórków.

  5. Do kalendarza jest milion gotowych paczek

Co do samego kodu. Niektóre funkcje/metody robią za dużo. Wprowadź trochę struktury. Dzielimy kod na

  • obiekty mutowalne + metody biznesowe charakterystyczne dla tych obiektów, oraz serwisy(klasy) z metodami bez stanu.
  • obiekty niemutowalne, funkcje tzw pure functions które zawierają logikę biznesową, oraz pozostałe z side efektami.

Do takiego podziału warto dołożyć testy! 

1
komentarz 19 maja 2021 przez adrian17 Ekspert (344,100 p.)

Do kalendarza jest milion gotowych paczek

To be fair, większość z tego miliona działa w sferze unixowych timestampów i nie obsłuży dat przed 1970. Tutaj też w zasadzie w ogóle nie robi nic z datami, tylko ma IMO lekko przekombinowaną abstrakcję na rok :)

+1 głos
odpowiedź 19 maja 2021 przez adrian17 Ekspert (344,100 p.)

HistoricalDate jest trochę przkombinowane i źle zaprojektowane.

Przede wszystkim, rozdziel ideę daty i długości czasu. Nie ma sensu "dodaj rok 12 naszej ery do roku 2000 naszej ery", znacznie więcej sensu ma "dodaj 12 lat do roku 2000 naszej ery".

Można by też ogólnie uprościć implementację, żeby pod spodem była jedna liczba reprezentująca rok, a na zewnątrz eksponować "faktyczny rok" i "faktyczną erę". Wtedy np dodawanie i porównywanie dat robi się trywialne i cała klasa skraca się o połowę (przy okazji zastąpiłem __le__ etc przez auto-generowane porównania i zamieniłem więcej metod na property):

@functools.total_ordering
class HistoricalDate:
    def __init__(self, year=None, era=Era.CE):
        assert year != 0 # albo exception
        year = year or date.today().year
        if era == Era.CE:
            self._year_counter = year
        else:
            self._year_counter = 1 - year

    @classmethod
    def _from_year_counter(cls, year_counter):
        ret = cls()
        ret._year_counter = year_counter
        return ret

    @property
    def year(self):
        if self._year_counter <= 0:
            return 1 - self._year_counter
        return self._year_counter

    @property
    def era(self):
        return Era.CE if self._year_counter > 0 else Era.BCE

    @property
    def century(self):
        return ((self.year - 1) // 100) + 1

    @property
    def millennium(self):
        return ((self.year - 1) // 1000) + 1

    def __str__(self):
        return f'{self.year} {self.era.name}'

    def add_years(self, years):
        return HistoricalDate._from_year_counter(self._year_counter + years)

    def __eq__(self, other):
        return self._year_counter == other._year_counter

    def __lt__(self, other):
        return self._year_counter < other._year_counter

Napisane na szybko :) Być może @dataclass by to mogło jeszcze bardziej uprościć, albo w ogóle zastąpić to jakąś zewnętrzną biblioteką...

komentarz 20 maja 2021 przez Ehlert Ekspert (212,630 p.)

Nigdy się nie przyzwyczaję. __get w php zaciemniał kod, kusił do tworzenia potworów bez typowania. Obecnie przydatne chyba tylko dla Twórców np. ORMów.

W Pythonie luzik, pod jakimś property dzieje się magia laugh

komentarz 20 maja 2021 przez adrian17 Ekspert (344,100 p.)
edycja 20 maja 2021 przez adrian17

Ale __get to coś zupełnie niezwiązanego? Jest bliższy Pythonowemu `__getattr__` (ale też nie do końca).

To jest bliższe np C#owym (lub JSowym) properties:

public int Century
{
     get { return ((Year - 1) // 100) + 1; }
}

// albo krocej:

public int Century => ((Year - 1) // 100) + 1;

// uzycie:
HistoricalDate date = //(...);
int century = date.Century;

Czy może o czymś innym mówisz?

komentarz 20 maja 2021 przez mattaha Użytkownik (690 p.)
Czy HistoricalEvent powinno dziedziczyć po HistoricalDate czy zastosować kompozycję tj. HistoricalDate jako atrybut obiektu HistoricalEvent?
komentarz 20 maja 2021 przez adrian17 Ekspert (344,100 p.)
Zdarzenie nie jest datą, zdarzenie ma datę.
komentarz 21 maja 2021 przez Ehlert Ekspert (212,630 p.)
Tak czy inaczej, nie widzę sensu ukrywania kodu pod wywołaniem property, skoro dzieje się tak coś więcej.

Podobne pytania

0 głosów
1 odpowiedź 4,176 wizyt
+1 głos
2 odpowiedzi 1,078 wizyt

92,454 zapytań

141,262 odpowiedzi

319,089 komentarzy

61,854 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...