Czy podzielić funkcje main() na mniejsze obiekty?

Witam, mam dylemat czy powinienem użyć programowania objektowego w moim pythonowym skrypcie. Podeśle kod funkcji main i byłbym wdzięczny za opinie czy w tym wypadku użyć takiego rozwiązania. Chciałbym się dowiedzieć czy było by to bardziej wydajne czy też bardziej czytelne rozwiązanie.
W kodzie jest dużo czytania jakichś konfiguracji z plików, których później będe używać w różnych etapach skryptu. Dlatego powstał mój dylemat czy jest to dobrze napisane, że tworze zmienne lokalne i wysyłam je jako argumenty w funkcjach. Dziękuje za wszelką pomoc!

def main():
    start_script_time = time.time()
    output_pipeClient.connect()

    config = ConfigParser()
    # w tym miejscu pobieram dane konfiguracyjne z pliku .ini, które następnie przesyłam jako argumenty do 
    # innych funkcji. Myślałem nad utworzeniem klasy i konstruktora, który by właśnie czytał ten plik i 
    # przypisywał te dane do zmiennych. To samo w dalszej częsci kodu - jakakolwiek zmienna była by 
    # inicjalizowana w konstruktorze.
    # Zamiast objektowości myślałem też czy te zmienne nie powinny być zmiennymi globalnymi. Nie musiałbym 
    # wtedy wielokrotnie przesyłać ich do innych funkcji
    try:
        config.read('config.ini')
        file_path = '{}'.format(config.get('ScriptConfig', 'file-path'))
        keywords_path = '{}'.format(config.get('ScriptConfig', 'key-words-path'))

        if ('{}'.format(config.get('ScriptConfig', 'key-words')) == 'True'):
            use_keywords = True
        else:
            use_keywords = False

        lang = '{}'.format(config.get('TesseractConfig', 'tesseract-lang'))
        tessdata_path = '{}'.format(config.get('TesseractConfig', 'tesseract-tessdata-path'))
        tesseract_exe_path = '{}'.format(config.get('TesseractConfig', 'tesseract-executable-path'))
        poppler_bin_path = '{}'.format(config.get('PopplerConfig', 'poppler-bin-path'))
    except:
        writeError("Error occured while reading config.ini.")  

    if tessdata_path != 'null':
        tessdata_dir_config = '--tessdata-dir "{}"'.format(tessdata_path)
    else:
        tessdata_dir_config = '' 

    if tesseract_exe_path != 'null':
            pytesseract.pytesseract.tesseract_cmd = tesseract_exe_path

    if (pytesseract.get_languages(tessdata_dir_config).count(lang) == 0):
        writeError("The specified language was not installed.")

    keywords = list
    # w tym miejscu zaczynam korzystać z moich funkcji. Nie ma ich zbyt wiele, ale myślałem np. czy funkcji 
    # generateWordsFromImg nie podzielić na pare osobnych części, żeby była lepsza czytelność.
    if use_keywords == True:
        keywords = loadKeywordsFromTxt(keywords_path)

    correct_files = loadFilesAndCheckFileFormat(file_path)

    words = generateWordsFromImg(correct_files, keywords, use_keywords, lang, tessdata_dir_config, poppler_bin_path)

    saveResultsToFile(words)

    end_script_time = time.time()
    script_time_seconds = end_script_time - start_script_time
    script_time_minutes = 0

    while(script_time_seconds >= 60):
        script_time_seconds -= 60
        script_time_minutes += 1

    final_time = "{}".format(script_time_minutes) + " min, " + "{:.2f}".format(script_time_seconds) + " sec."
    output_pipeClient.write("COMPLETE, check results.txt file " + "| Time: " + (str)(final_time))
    sys.exit()

Myślę, że zagubiłeś się w myśleniu, że w ogóle potrzebujesz już ileś zmiennych. Przecież wystarczy ci słownik.

 # w tym miejscu zaczynam korzystać z moich funkcji

Tylko, że twój problem nie zaczyna się tutaj, tylko wcześniej. W bloku try.

Ogólnie jak rozumiem, to masz obiekt config, z którego wyłapujesz zmienne. 1 wpis w konfigu = 1 zmienna.

I tu myślę trzeba się cofnąć. Po co tworzysz zmienne lokalne? Czemu nie utworzysz sobie słownika? Jeśli tak naprawdę nie chcesz mieć tych zmiennych osobno, tylko razem, bo potrzebujesz podawać razem ileś zmiennych:

words = generateWordsFromImg(correct_files, keywords, use_keywords, lang, tessdata_dir_config, poppler_bin_path)

tak to mógłbyś po prostu podać słownik z configiem i tyle.

Choć z drugiej strony nie trzeba szaleć i jak jakaś funkcja korzysta tylko z jednej zmiennej np.

loadFilesAndCheckFileFormat(file_path)

to nie trzeba jej podawać całego słownika, tylko np. my_config['file_path'] zakładając, że my_config to byłby ten słownik właśnie.

No i co naprawdę te funkcje robią? Być może i te funkcje mogłyby być napisane lepiej (tak zgaduję, bo coś przypuszczam, że i tam może być ukryty problem).

Myślę, że za wcześnie sięgasz po obiektówkę i że przypuszczalnie można by użyć OOP, jednak twój problem zaczyna się gdzie indziej i OOP nie rozwiązałoby twojego podstawowego problemu, a tylko trochę zamiotło pod dywan.

LukeJL napisał(a):

Myślę, że zagubiłeś się w myśleniu, że w ogóle potrzebujesz już ileś zmiennych. Przecież wystarczy ci słownik.

 # w tym miejscu zaczynam korzystać z moich funkcji

Tylko, że twój problem nie zaczyna się tutaj, tylko wcześniej. W bloku try.

Ogólnie jak rozumiem, to masz obiekt config, z którego wyłapujesz zmienne. 1 wpis w konfigu = 1 zmienna.

I tu myślę trzeba się cofnąć. Po co tworzysz zmienne lokalne? Czemu nie utworzysz sobie słownika? Jeśli tak naprawdę nie chcesz mieć tych zmiennych osobno, tylko razem, bo potrzebujesz podawać razem ileś zmiennych:

words = generateWordsFromImg(correct_files, keywords, use_keywords, lang, tessdata_dir_config, poppler_bin_path)

tak to mógłbyś po prostu podać słownik z configiem i tyle.

Choć z drugiej strony nie trzeba szaleć i jak jakaś funkcja korzysta tylko z jednej zmiennej np.

loadFilesAndCheckFileFormat(file_path)

to nie trzeba jej podawać całego słownika, tylko np. my_config['file_path'] zakładając, że my_config to byłby ten słownik właśnie.

No i co naprawdę te funkcje robią? Być może i te funkcje mogłyby być napisane lepiej (tak zgaduję, bo coś przypuszczam, że i tam może być ukryty problem).

Myślę, że za wcześnie sięgasz po obiektówkę i że przypuszczalnie można by użyć OOP, jednak twój problem zaczyna się gdzie indziej i OOP nie rozwiązałoby twojego podstawowego problemu, a tylko trochę zamiotło pod dywan.

Dzięki za tą opcję ze słownikiem, będe musiał to sprawdzić na pewno i myślę, że rozwiązało by to mój problem. Wydaje mi się, że reszta kodu jest napisana dobrze, bo te funkcje nie są skomplikowane i robią w sumię tylko kilka rzeczy.

loadFilesAndCheckFileFormat, sprawdza ścieżke do plików, lub konkretne podane pliki (w zależności od wyboru użytkownika), czy mają poprawne rozszerzenia, czyli konkretnie .png .jpg .gif .pdf. Jeśli ścieżka do pliku jest poprawna, dodaje ją do dictionary wraz z typem, czyli img lub pdf

następnie jest funkcja generateWordsFromImg, w której jest pętla, która sprawdza każdy plik w dictionary i w zależności od typu, jak scieżka jest typu pdf, to konwertuje go na zdjecia przez pdf2image, a jak scieżka ma typ img, to otwieram to zdjęcie PIL'em. Zdjęcia zapisuje w liście. Następnie z każdego ze zdjęć generuje tekst poprzez OCR i w zależności czy use_keywords jest true albo false, to albo szuka w tekście tylko słów kluczowych i zlicza ich wystąpienia, lub po prostu zlicza wystąpienia wszystkich słów ze zdjęć.

saveResultsToFile(words) zapisuje do pliku wyniki.

writeError - używam tej funkcji w każdym wychwyceniu błedu. Funkcja ta zapisuje cały dokładny kod błedu, wraz z własnym komentarzem do pliku txt.

loadKeywordsFromTxt - wczytuje listę słów kluczowych z pliku txt.

Z takich zmiennych, które deklaruje w funkcjach, to są w loadFilesAndCheckFileFormat: dir_path - string, files_paths - lista, correct_files - dict.

Później w generateWordsFromImg mam images - lista, text - string - tekst, który wygenerował ocr, words_founded - lista, words_counts - dict.

I to jest tak w zasadzie wszystko, co robi ten skrypt. Całość ma 227 linii kodu.

Jest też kilka innych rzeczy, np obiekt output_pipeClient i obiekt progressBarInfo_pipeClient, które bazują na klasie PipeClient z innego pliku, w której się bawiłem właśnie troche w OOP. W tej klasie jest tworzony owy pipe client, który komunikuje się po procesie z gui napisanym w c#

Z tych ważniejszych zmiennych to w sumie tyle.

Może było by lepiej jakbym podał po prostu cały kod, który niest jest bardzo skomplikowany, ale myślę, że nawet okej to rozpisałem.

Tu jest kod jednej z tych funkcji.

def loadFilesAndCheckFileFormat(dir_path):
    correct_files = dict()
    files_paths = list()

    #specified files selected
    if(dir_path == "null"):
        try:
            filespaths_txt = open("filespaths.txt", "r", encoding="utf-8")    
        except: 
            writeError("Error occured with filespaths file.")

        for line in filespaths_txt.readlines():
            size = len(line)
            files_paths.append(line[:size - 1])
        filespaths_txt.close()
    #entire folder search selected
    else:
        try:
            files_names = os.listdir(dir_path)
        except:
            writeError("Error occured while loading files. Check is file path correct.", dir_path)

        for file_name in files_names:
            files_paths.append(os.path.join(dir_path, file_name))

    #check files extensions are correct
    for path in files_paths:
        if path.endswith('.png') or path.endswith('.jpg') or path.endswith('.jpeg'):
            correct_files[path] = 'img'
        elif path.endswith('.pdf'):
            correct_files[path] = 'pdf'
        else:
            print(path, "- wrong file format")

    return correct_files

I funkcja generateWordsFromImg działa na tej samej zasadzie. Przetwarza coś a potem zwraca wynik, który jest potrzebny do innej funkcji.

Programowanie obiektowe to nie jest po prostu używanie obiektów, tak samo jak programowanie funkcyjne to nie jest po prostu używanie funkcji.

Być może chodzi Ci o to, czy jest sens podzielić Twoją funkcję main() na mniejsze obiekty? Tylko to nie jest programowanie obiektowe jeszcze.

Riddle napisał(a):

Programowanie obiektowe to nie jest po prostu używanie obiektów, tak samo jak programowanie funkcyjne to nie jest po prostu używanie funkcji.

Być może chodzi Ci o to, czy jest sens podzielić Twoją funkcję main() na mniejsze obiekty? Tylko to nie jest programowanie obiektowe jeszcze.

Myślałem żeby zrobić coś na zasadzie jak poniżej. Zrobiłbym plik generate.py gdzie była by klasa i opakowane te metody, a zmienne konfiguracyjne bym inicjalizował w konstruktorze (zamieściłbym tam cały odczyt tego config.ini). Potem plik main.py gdzie stworze obiekt i będe korzystał już z konkretnych metod.
Zamiast czegoś takiego: "words = generateWordsFromImg(correct_files, keywords, use_keywords, lang, tessdata_dir_config, poppler_bin_path)",
gdzie przypisuje do zmiennej wynik funkcji, deklarowałbym tą zmienną w tej klasie i inicjalizowałbym ją w tej właśnie metodzie. A później gdy będzie chciał wywołać metodę saveResultsToFile, to napisze ją na tej zasadzie: saveResultsToFile(self) i w środku użyje self.words. Czy ma to sens?

Ten kod poniżej postanowiłem napisać jako klasę z uwagi na to, że potrzebuje dwa różne "Pipe'y" z którymi się będe łączył. A w przypadku co opisywałem powyżej będe potrzebował w zasadzie tylko jeden obiekt. Stąd mam dylemat :D

import win32file
import win32pipe

class PipeClient:
    pipe_handle = None
    pipe_name = None

    def __init__(self, name):
        self.pipe_name = name
        self.pipe_handle = win32pipe.CreateNamedPipe(
                r'\\.\pipe\\'+name,
                win32pipe.PIPE_ACCESS_DUPLEX,
                win32pipe.PIPE_TYPE_MESSAGE | win32pipe.PIPE_READMODE_MESSAGE | win32pipe.PIPE_WAIT,
                1, 65536, 65536,
                0,
                None)

    def connect(self):
        win32pipe.ConnectNamedPipe(self.pipe_handle, None)
    
    def disconnect(self):
        win32pipe.DisconnectNamedPipe(self.pipe_handle)
        win32file.CloseHandle(self.pipe_handle)
    
    def write(self, var):
        ret, length = win32file.WriteFile(self.pipe_handle, (var + '\n').encode())
        win32file.FlushFileBuffers(self.pipe_handle)