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
  • Najlepsze
  • Wszystkie komentarze
szubke
szubke
@ozzi91: dodaj tam jeszcze warunek isPies i nie ma co reszty refactoryzowac, lepsze nie bedzie ( ͡° ͜ʖ ͡° )つ──☆*:・゚
FredOnizuka
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 :)
zibizz1
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.
mrq
mrq
1
@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
TDDwBashu
  • adult
1
Treść przeznaczona dla osób powyżej 18 roku życia...
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
ozzi91
ozzi91 Autor
0
@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,
ozzi91
ozzi91 Autor
0
@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
ozzi91 Autor
0
@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ć?
ozzi91
ozzi91 Autor
0
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
ozzi91
ozzi91 Autor
0
@TDDwBashu aha i co najważniejsze, dziękuję bardzo za odzew, dużo to dla mnie znaczy