[ozzi91]

#programowanie #programista15k #python

Ma to sens pod kątem budowy? Wiadomo, nic tu napisane za bardzo nie jest ale robię mały refactor kodu z całkowicie odmiennym podejściem gdzie sama funkcjonalność jest ale wymaga dobrego wzoru.

Docelowo widzę to tak, że user z listy wybiera jakąś opcję, tworzy sie obiekt klasy Document, w zależnosci co chce tak naprawdę zrobić są używane określone metody, dostaje pytanie czy chce wygenerować docx, odpowiada tak więc mamy orange.generate() czy tam czy chce wysłać maila no to orange.sendemail(). I wiadomo ja kwybierze Orange no to są dodawane poszczególne replacementy jak w tym przykładzie, zapewne można to zrobić jakąś jedną komendą zamiast powtarzać .addreplacement (jak wiecie jak to dajcie znać) ale nie jest to jakoś bardzo istotne na tą chwilę.

import os
from urllib.parse import quote
import docx
from python_docx_replace import docx_replace
import docx2pdf

class Document:
    def __init__(self, help_name, file_name, template_file, directory_path):
        self.help_name = help_name
        self.file_name = file_name
        self.template_file = template_file
        self.directory_path = directory_path
        self.replacements = {}
        self.wp_req = False
        self.pelnomocnictwo_req = False
        self.pdf_req = False

    def add_replacement(self, key, value):
        self.replacements[key] = value

    def generate(self):
        doc = docx.Document(self.template_file)
        docx_replace(doc, **self.replacements)
        output_path = "wypelnione.docx"
        doc.save(output_path)
        if self.pdf_req == True:
            docx2pdf.convert(output_path)
        else:
            pass

    def save(self):
        # Logic to save the generated document to the specified directory.
        pass

    def send_email(self, initial_name: str, toaddrs: str, subject: str, msg_content: str, attachment_list: list[str]):
        pass

wnioski_number = 7
orange = Document("Zarząd Drogowy", "PR do ZD - Wniosek o uzgodnienie lokalizacji ", "template.docx", os.path.join(f"{wnioski_number}. Wnioski", "ZD"))
orange.add_replacement("today_date", "2023-10-23")
orange.add_replacement("title", "Budowa drogi")
orange.add_replacement("city", "Kraków")
orange.add_replacement("street", "Sławkowska")
orange.wp_req = True
orange.pelnomocnictwo_req = True

orange.generate()

to samo co wyżej na https://pastebin.com/MdUHeM6k

[zibizz1]

@ozzi91
Jak nikt tego potem nie modyfikuje to zawsze używam
https://handlebarsjs.com/
I template jest w HTML i wrzucam wszystkie dane jako slownik do renderowania.

[szubke]

@ozzi91: dodaj tam jeszcze warunek isPies i nie ma co reszty refactoryzowac, lepsze nie bedzie ( ͡° ͜ʖ ͡° )つ──☆*:・ﾟ

[FredOnizuka]

Umieszczenie w tej klasie metody "send_email", jakoś za bardzo mi nie pasuje. Tam powinny być tylko metody dotyczące manipulacji samym dokumentem, a trudno do nich zaliczyć wysyłanie maila. Ale, oczywiście można - serwer od tego raczej nie wybuchnie :)

[mrq]

@ozzi91: nie wiem czy to nie za daleko w las ale poczytać sobie zawsze możesz :)
https://refactoring.guru/pl/design-patterns/creational-patterns

Dobra rozrywka może być próba przełożenia tych wzorców na swoje potrzeby bo że tak powiem tylko ty widzisz szerszy kontekst swojego programu.

Co do samej klasy to zmieniłbym tylko nazwę metody z add_replacements na samo replace

[TDDwBashu]

@ozzi91: #!$%@?łeś tych argumentów do inita - po co? Aby się gorzej testowało???

Co by się takie stało jakby def generate(self): przyjmowało np. template_file?

Jak masz tyle argumentów w init to robisz to źle.

send_email totalnie z dupy w tej klasie.

orange.wp_req = True
orange.pelnomocnictwo_req = True

Totalnie nic nie mówi. Daj to do klasy z odpowiednimy metodami.

Zamiast robić:

orange.add_replacement("today_date", "2023-10-23")

Czemu po prostu nie dodasz słownika?

[ozzi91]

@zibizz1 docelowo wartości slownika będą skądś pobierane, w mojej pracy króluje Excel wiec nie wiem czy nie z jakiegoś arkusza-jak macie jakieś lepsze rozwiązanie to dajcie znać.

@szubke panie zaczynam dopiero, robię to póki co hobbistycznie więc takie uwagi niewiele wnoszą...

@FredOnizuka yep, wywalę to

@mrq super, poczytam

@TDDwBashu myślałem że w init powinno być jak najwięcej wspólnych elementów a najlepiej wszystkie - wydawało mi się to logiczne ale wychodzi na to

[ozzi91]

@TDDwBashu a co do słownika - myslalem że tak będzie czytelniej, tymbardsiej że poszczególne wartości kluczy będą się zmieniać, będą skądś tam wczytywnae

[ozzi91]

@TDDwBashu:

Co by się takie stało jakby def generate(self): przyjmowało np. template_file?

jednak tego nie rozumiem, o co dokładnie Ci chodzi? możesz mi jakiś snipp kodu wkleić?

[TDDwBashu]

@ozzi91:

myślałem że w init powinno być jak najwięcej wspólnych elementów a najlepiej wszystkie

Im mniej w init tym lepiej. Pomyśl z perspektywy testowania.
Chcesz sobie przetestować add_replacement. Na prawdę chcesz ładować te wszystkie argumenty do klasy, aby ją zainicjować i przetestować metode?

a co do słownika - myslalem że tak będzie czytelniej, tymbardsiej że poszczególne wartości kluczy będą się zmieniać, będą skądś tam wczytywnae

Nie jest czytelniej, bo masz

[ozzi91]

Im mniej w init tym lepiej. Pomyśl z perspektywy testowania. Chcesz sobie przetestować addreplacement. Na prawdę chcesz ładować te wszystkie argumenty do klasy, aby ją zainicjować i przetestować metode?

Ok, rozumiem.

Nie jest czytelniej, bo masz X linii zamiast jednej. Zawsze możesz dodać metodę updatereplacement czy coś, która będzie aktualizowała słownik w razie potrzeby. Znowu - patrz z perspektywy testowania.

i

Patrząc szerzej - po co chcesz aktualizować dokument (z add

[ozzi91]

@TDDwBashu aha i co najważniejsze, dziękuję bardzo za odzew, dużo to dla mnie znaczy

[TDDwBashu]

@ozzi91: Ciężko dać jakąś opinie bez szerszego kontekstu.

To, że klasa BaseTemplate jest "pusta" jest OK. Poczytaj o interface - https://realpython.com/python-interface/. Co prawda jest to termin bardziej z Javy, ale Python to dalej język obiektowy.
Ogólnie traktuj BaseTemplate jako wzorzec/templatke dla klas dziedziczących.

Nie musisz na każdą logikę tworzyć klasy.
Jak send_email będzie po prostu metodą bez klasy - to po prostu będzie spoko. Obiekt jak nazwa wskazuje opisuje coś. Obiekt